Fix escapes in raw template payloads - #8630
Conversation
Signed-off-by: Christoph Knittel <ck@cca.io>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Signed-off-by: Christoph Knittel <ck@cca.io>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e5f9b0419
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ] -> | ||
| Some | ||
| (source.txt, Bs_flow_ast_utils.flow_deli_offset (Some "js"), expression) | ||
| let source = String_literal.decode_raw_template_source source.txt in |
There was a problem hiding this comment.
When malformed embedded JavaScript occurs after a decoded escape, the Flow parser reports positions in the shortened source, but check_flow_errors later projects those positions onto the original ReScript text using only a constant delimiter offset. For example, an error following \`` or ${` is highlighted one or more columns early, while an error after a removed backslash-newline continuation can be reported on the wrong line. The decoder needs to retain an original-offset mapping for diagnostics, or error locations must otherwise be translated before reporting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks for catching this. I checked the ReScript 9.1.4 implementation: parseTemplateStringLiteral already decoded these escapes before the Flow parser ran, while check_flow_errors projected the resulting locations using only the same fixed delimiter offset. So diagnostic drift after a removed escape or line continuation is pre-existing behavior rather than something introduced by this change. Preserving exact positions would require carrying a source-offset map through decoding (including Unicode and removed line terminators), which is a substantially broader diagnostics change and outside the scope of this PR. I’d prefer to keep this PR focused on restoring the raw-template escape behavior.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8630 +/- ##
==========================================
- Coverage 77.38% 77.38% -0.01%
==========================================
Files 474 474
Lines 63524 63553 +29
==========================================
+ Hits 49156 49178 +22
- Misses 14368 14375 +7
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
2 participants
Summary
Fix a regression introduced in ReScript 10 where escapes used inside backquoted
%raw,%ffi, and%repayloads leaked into the emitted JavaScript.Escapes required by the surrounding ReScript template—such as escaped backticks, dollar signs, backslashes, and line continuations—are removed before parsing and emitting the embedded JavaScript. JavaScript-specific escapes such as
\nand\x01remain intact.Add unit and end-to-end regression coverage for escaped JavaScript template literals.
For example:
%raw(`const message = \`Hello, \${name}\``)is emitted as:
The JavaScript template literal remains intact, while the ReScript-specific escaping is removed before emission.
Fixes #6236.