-
Notifications
You must be signed in to change notification settings - Fork 471
Stop mangling tagged templates and backquoted strings #7841
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
Conversation
8459260
to
89cb764
Compare
89cb764
to
a16c424
Compare
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the compilation of tagged templates and backquoted strings in ReScript to preserve their original formatting in the generated JavaScript output. Previously, these were being mangled with unnecessary escape sequences.
- Adds support for
DBackQuotes
delimiter type to preserve template literal syntax - Updates the string constant representation to track delimiter information instead of just unicode flag
- Modifies the compiler pipeline to properly handle backquoted strings and tagged templates
Reviewed Changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/tests/src/*.mjs | Test output files showing proper backquote preservation in generated JS |
compiler/frontend/external_arg_spec.ml* | Adds DBackQuotes delimiter type |
compiler/frontend/ast_utf8_string_interp.ml | Updates string interpolation handling for template literals |
compiler/frontend/lam_constant.ml* | Changes Const_string to use delim field instead of unicode boolean |
compiler/core/*.ml | Updates constant handling throughout compiler pipeline |
compiler/core/js_dump.ml | Adds backquote output support in JS generation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
let is_template = | ||
Ext_list.exists e.pexp_attributes (fun ({txt}, _) -> | ||
match txt with | ||
| "res.template" | "res.taggedTemplate" -> true | ||
| _ -> false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The template detection logic is duplicated in the transform_exp
function but not in transform_pat
. This inconsistency could lead to different behavior between expressions and patterns. Consider extracting this logic into a helper function or ensuring consistent template detection across both functions.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is because you can't have string interpolation in pattern matching.
let is_unicode_string opt = Ext_string.equal opt Delim.escaped_j_delimiter | ||
let is_unicode_string opt = | ||
Ext_string.equal opt Delim.escaped_j_delimiter | ||
|| Ext_string.equal opt Delim.escaped_back_quote_delimiter | ||
|
||
let is_unescaped s = Ext_string.equal s Delim.unescaped_js_delimiter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The is_unescaped
function only checks for the unescaped_js_delimiter
but doesn't account for the new backquote delimiter. This could lead to inconsistent behavior when processing unescaped strings with different delimiters.
let is_unescaped s = Ext_string.equal s Delim.unescaped_js_delimiter | |
let is_unescaped s = | |
Ext_string.equal s Delim.unescaped_js_delimiter | |
|| Ext_string.equal s Delim.escaped_back_quote_delimiter |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested against a large real-world project and worked fine.
Fixes #7777, #7196.
With this PR
generates
It used to generate
Backquoted strings are now compiled as backquoted strings in JS, but they are still concatenated with
+
when there are interpolations, this could be improved in a subsequent PR (for v12.1?).