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

Port j quoted strings interpolation from Melange #139

Merged
merged 2 commits into from
Jun 25, 2024
Merged

Conversation

jchavarri
Copy link
Contributor

@jchavarri jchavarri commented Jun 19, 2024

Melange does a bunch of things with these quoted strings.

To ease maintenance and prevent server rendering mismatches, the version of melange.ppx in server-reason-react could error out. Edit: see comment below.

Partially fixes #139

@jchavarri jchavarri changed the title Fail on j and js quoted strings Port j quoted strings interpolation from Melange Jun 20, 2024
@jchavarri
Copy link
Contributor Author

After trying the original approach in this PR (not support j and js in universal code) we realized it's currently not possible to define string literals with unicode characters in a way that both OCaml and Melange can understand (see related melange-re/melange#1141).

The solution then to have "universal unicode strings" has switched to supporting j interpolation in native, which 18940af does. Thanks to @anmonteiro for the help figuring out the details.

Quoted strings with js used in universal code can be left as is: in OCaml any unicode characters added to these literals will be printed just fine, like any other string in OCaml.

@jchavarri
Copy link
Contributor Author

jchavarri commented Jun 25, 2024

Discussed with @davesnx offline, we can merge this and keep iterating if anything's missing.

@jchavarri jchavarri merged commit 036a4f4 into main Jun 25, 2024
2 checks passed
@jchavarri jchavarri deleted the fail-on-j-and-js branch June 25, 2024 11:03
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.

None yet

1 participant