-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TypeError: window.Dashboard.revive_payload is not a function #241
Comments
@hhaensel can you please check this - if you can fix quickly great, otherwise we need to have a new release to rollback the change until it's backwards compatible. |
I will also take a look as the CDN should use the versioned script, not the master. |
Thank you for the quick response. Re. versioning, agreed. But I misspoke -- this was a patch version difference (eg, I was on 0.27.0 and the breaking release was 0.27.18) Since I updated to the latest Stipple, I noticed another bug in the Sessions from some recent changes. I'll open the issue with the git blame. |
@svilupp May I ask, when If you provide a snippet of your script, I could either propose a solution (most probably it can be simply left out) or I could bring back the function in a new version of Stipple. |
Oh, I begin to understand. @essenciary is probably right that the CDN version and the Stipple version don't fit. |
I am not calling the function directly. It’s a normal Stipple dashboard on Stipple 0.27.0, which calls revive_* as part of the JavaScript codegen. It was an issue only when running in prod env, when the scripts are sourced from CDN, so perhaps the issue is simply that it wasn’t versioned. My Stipple locally created model that required the function, but it was no longer available on the CDN. —— |
I just checked that there are, indeed, different (and correct) versions of watchers.js on
and
I suspect that you have deved Stipple and are running from master. In that case Stipple will call
and you will run in the difficulties above. using Pkg
ver = filter(x-> x.second.name == "Stipple", Pkg.dependencies()) |> x -> first(x)[2].version
Genie.Assets.assets_config!(Stipple, version = string(ver)) |
@essenciary could it be that we always use |
We might add function version_string(package::String)::String
endswith(package, ".jl") && (package = String(package[1:end-3]))
pkg_dict = filter(x -> x.second.name == package, Pkg.dependencies())
isempty(pkg_dict) ? "master" : string(first(pkg_dict)[2].version)
end
version_string(m::Module) = version_string(String(nameof(m)))
function Genie.Assets.AssetsConfig(m::Module)
package = "$(nameof(m)).jl"
version = string(package_version(m))
Genie.Assets.AssetsConfig(; package, version)
end
function Genie.Assets.AssetsConfig(package::String)
version = version_string(package)
Genie.Assets.AssetsConfig(; package, version)
end so that we then can Genie.Assets.AssetsConfig("Stipple.jl")
# or
Genie.Assets.AssetsConfig(Stipple) That would be breaking, though, because the files would no longer reside in master but in the respective branch |
@svilupp @essenciary We should consider the above changes for the next minor Stipple release ... |
https://cdn.statically.io/gh/genieframework/stipple.jl/master/assets/js/watchers.js is not yet updated, but https://cdn.statically.io/gh/genieframework/stipple.jl/v0.27.19/assets/js/watchers.js is already available. |
FWIW, I believe that was driver of my issue. I didn’t clock that. It’s always set for version=“master”: https://github.com/GenieFramework/Genie.jl/blob/2128aa0147770f5944af4b46d7a902ae88a24083/src/Assets.jl#L25C10-L25C10 I didn’t dev the package, that was the behaviour on 0.27.0 that was added as a normal package. |
Yes, using |
OK - I pushed on master... Not sure how to test the tagged assets without actually tagging the Genie release first. So I'll do that. |
@hhaensel thank you very much for the fix! |
You need to modify: isa(package, Module) && (package = string(package)) to isa(package, Module) && (package = String(nameof(package))) because EDIT: In our standard use cases it will probably not play a role, but if people might use it for other purposes it would be nice if the function gives a correct value. |
@hhaensel got it! I'll update and release |
@svilupp can you please update to latest Genie and Stipple and check if everything works as expected? You should now be getting versioned assets. |
Thank you for the quick turnaround. It all worked fine on Stipple.jl 0.27.18. I get the following errors:
Scripts seem to be properly injected into the main js file:
However, they do not seem available on the CDN. I get the following when I try to open the files directly:
|
Oh ugh, the CDN does not support URLs with Git tags. |
OK, it works but it needs the |
@svilupp releasing new patch versions. Sorry for the issues and thanks for the info. |
see above 😉 it had the 'v' already. The master is now updated, so any version should currently run with master as well |
@hhaensel Mmmm yes... but not the version I initially released... |
master should be the current master, right? |
FWIW, I can confirm that the latest master works well. This issue can be closed from my perspective. |
@svilupp thanks for confirming, closing. |
First, thank you for the awesome package.
I suspect that this PR is breaking for all Stipple apps running on previous versions in ENV=PROD.
The reason is that our mini app has stopped working after the release with the following error:
The local version that uses corresponding JS helper files works fine. The PROD version that grabs scripts from GenieFramework CDN throws the above error in the browser console.
Is there a way to add an alias to be backwards compatible?
It's a minor version release, so I wouldn't expect it to be breaking.
The text was updated successfully, but these errors were encountered: