Skip to content
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

Fix print_script for 1.10 #36

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Conversation

j-fu
Copy link
Contributor

@j-fu j-fu commented Sep 29, 2023

Appearantly, at the time when @generated is evaluated, methods from other packages may not have been compiled yet, so
hasmethod(show, Tuple{IO, MIME{Symbol("text/javascript")}, value}) returns false, and the @generated function print_script throws an error.

As the old version worked in 1.9.3, the different behavior appears to be the result of subtle changes in precompilation/initialization behavior somewhere between 1.9.3 and 1.10.0-beta2.

Appearantly, at the time when @generated is evaluated, methods from
other packages may not have been compiled yet, so
`hasmethod(show, Tuple{IO, MIME{Symbol("text/javascript")}, value})`
returns false, and the @generated function print_script throws an error.

As the old version worked in 1.9.3, this appears to be the result of
subtle changes in precompilation/initialization
behavior somewhere between 1.9.3 and 1.10.0-beta2.
@j-fu
Copy link
Contributor Author

j-fu commented Sep 29, 2023

@j-fu
Copy link
Contributor Author

j-fu commented Sep 29, 2023

Working MWE for this PR

@j-fu
Copy link
Contributor Author

j-fu commented Sep 29, 2023

The change in behavior appears to be due to the new parser.

When setting export JULIA_USE_FLISP_PARSER=1 the original MWE using the released version of HypertextLiteral works.

@clarkevans
Copy link
Collaborator

clarkevans commented Sep 29, 2023

I'm curious does the new parser fix JuliaLang/julia#18221 ? If so we could report it as being fixed.

Regardless, could wrap this in a version check? This code is there to reduce user confusion.

@j-fu
Copy link
Contributor Author

j-fu commented Sep 29, 2023

No it doesn't solve JuliaLang/julia#18221 .
As for the generated function based workaround, julia docs state that generated functions should not use hasmethod. So the new parser seems to be in accord with the docs here.

Will see if this can be wrapped into a version check. But in 1.10 we are on field 1 anyway.

@j-fu
Copy link
Contributor Author

j-fu commented Sep 29, 2023

What about implementing

Base.show(io::IO, ::MIME"text/javascript", value::Any)=error("""Missing show(::IO,::MIME"text/javascript",::$(typeof(value))""")

Seems to be a bit nasty as well, but might work (did not test this yet outside of the REPL).

@clarkevans
Copy link
Collaborator

clarkevans commented Sep 29, 2023

That would be type thievery?

Keep the old behavior (via @generated) for versions <=1.9.x.
@j-fu
Copy link
Contributor Author

j-fu commented Sep 30, 2023

I think not exactly, but kind of scary anyway.

I added the version discriminator as you proposed. I think this is ready to merge.

@clarkevans
Copy link
Collaborator

I suppose there's not much else we could do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants