-
-
Notifications
You must be signed in to change notification settings - Fork 737
feat(transformer): support tagged template expression with </script transformation
#15664
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
feat(transformer): support tagged template expression with </script transformation
#15664
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
CodSpeed Performance ReportMerging #15664 will not alter performanceComparing Summary
Footnotes
|
97af8ab to
7745501
Compare
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 implements a transformation plugin for tagged template expressions containing </script> tags to prevent script tag parsing issues when JavaScript is embedded in HTML. The transformation converts tagged template literals into function calls using cached template objects via a Babel helper.
Key Changes
- Adds a new
TaggedTemplateTransformplugin that detects and transforms tagged templates containing</script>(case-insensitive) - Integrates the plugin into the transformer pipeline with proper option handling across napi/TypeScript/Rust boundaries
- Includes comprehensive test fixtures covering basic cases, expressions, case variations, and edge cases
Reviewed Changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/oxc_transformer/src/plugins/tagged_template_transform.rs | Core plugin implementation with script tag detection and transformation logic |
| crates/oxc_transformer/src/plugins/mod.rs | Plugin registration and integration into the traversal pipeline |
| crates/oxc_transformer/src/plugins/options.rs | Plugin options structure definition |
| crates/oxc_transformer/src/options/mod.rs | Option mapping from Babel configuration |
| crates/oxc_transformer/src/options/babel/plugins.rs | Babel plugin entry parsing for "tagged-template-transform" |
| crates/oxc_transformer/src/common/helper_loader.rs | Added TaggedTemplateLiteral helper enum variant |
| napi/transform/src/transformer.rs | NAPI bindings for plugin options |
| napi/transform/index.d.ts | TypeScript definitions for plugin configuration |
| npm/runtime/src/helpers/esm/taggedTemplateLiteralEscape.js | Runtime helper function (currently unused) |
| tasks/transform_conformance/src/constants.rs | Added plugin to conformance test suite |
| tasks/transform_conformance/snapshots/* | Updated test snapshots showing improved pass rates |
| tasks/transform_conformance/tests/plugin-tagged-template-transform/* | Comprehensive test fixtures for various scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/oxc_transformer/src/plugins/tagged_template_transform.rs
Outdated
Show resolved
Hide resolved
crates/oxc_transformer/src/plugins/tagged_template_transform.rs
Outdated
Show resolved
Hide resolved
crates/oxc_transformer/src/plugins/tagged_template_transform.rs
Outdated
Show resolved
Hide resolved
</script transformation
|
@Dunqing in case you weren't aware, there's a panic in conformance. |
Sorry, I forgot to push the local changes. |
5a07258 to
1d1dc5a
Compare
sapphi-red
left a comment
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.
LGTM on my side
1d1dc5a to
0f876fe
Compare
|
I'm afraid I don't think this is right. I'll make sure I'm correct about that, and write up what I think the problem is. In meantime, have rebased on latest main + added one formatting nit commit. |
|
What we're missing is a 2nd array for raw strings if Input: foo`</script>`;
foo`</script>\n`;
foo`</script>\u`; // No cooked because invalid escape `\u`Current output: // Correct
foo(_temp || (_temp = _taggedTemplateLiteral(["</script>"])));
// `raw` property incorrect in `foo`
foo(_temp2 || (_temp2 = _taggedTemplateLiteral(["</script>\n"])));
// Syntax error!
foo(_temp3 || (_temp3 = _taggedTemplateLiteral(["</script>\u"])));Output should be: foo(_temp || (_temp = _taggedTemplateLiteral(["</script>"])));
foo(_temp2 || (_temp2 = _taggedTemplateLiteral(["</script>\n"], ["</script>\\n"])));
// ^^^^^^^^^^^^^^^^^^
foo(_temp3 || (_temp3 = _taggedTemplateLiteral([void 0], ["</script>\\u"])));
// ^^^^^^^^^^^^^^^^^^^^^^^^^^Because this only transforms tagged template literals containing Additionally, is this transform enabled in benchmarks? I think it may be a perf regression. We should check if |
|
Actually, this transform is enabled in benchmarks. Effect is only -1%. That's acceptable for now. We can improve it in a follow-up. |
|
|
||
| let raw_bytes = raw.as_bytes(); | ||
| // Get the bytes up to the last possible starting position of the script tag | ||
| let max_remain_len = raw_bytes.len().saturating_sub(SCRIPT_TAG.len()); |
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.
No need for saturating_sub here. You already checked that len < SCRIPT_TAG.len() above, so just raw_bytes.len() - SCRIPT_TAG.len() would do (cheaper).
| for (idx, byte) in raw_bytes_iter { | ||
| if byte == b'<' | ||
| && SCRIPT_TAG | ||
| .iter() | ||
| .zip(raw_bytes[idx..].iter()) | ||
| .all(|(a, b)| *a == b.to_ascii_lowercase()) | ||
| { | ||
| return true; | ||
| } | ||
| } |
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.
We have a faster way to check for </script in oxc_codegen here:
oxc/crates/oxc_codegen/src/str.rs
Lines 726 to 743 in 3b548dc
| /// Check if `slice` is `</script`, regardless of case. | |
| /// | |
| /// `slice.len()` must be 8. | |
| // | |
| // `#[inline(always)]` so that compiler can see from caller that `slice.len() == 8` | |
| // and so `slice.try_into().unwrap()` cannot fail. This function is only 4 instructions. | |
| #[expect(clippy::inline_always)] | |
| #[inline(always)] | |
| pub fn is_script_close_tag(slice: &[u8]) -> bool { | |
| // Compiler condenses these operations to an 8-byte read, u64 AND, and u64 compare. | |
| // https://godbolt.org/z/K8q68WGn6 | |
| let mut bytes: [u8; 8] = slice.try_into().unwrap(); | |
| for byte in bytes.iter_mut().skip(2) { | |
| // `| 32` converts ASCII upper case letters to lower case. | |
| *byte |= 32; | |
| } | |
| bytes == *b"</script" | |
| } |
overlookmotel
left a comment
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.
We've decided to merge, and fix the bug + perf in follow-ups.
Merge activity
|
… transformation (#15664) Implement a transformation plugin for tagged template expressions containing </script> tags to prevent script tag issues in browser environments. This plugin transforms tagged template literals containing `</script` (case-insensitive) to use a helper function call with template caching, matching esbuild's behavior. ### Example Transforms tagged templates to use cached template objects with a helper function: Input: ```js foo`</script>` bar`<script>${content}</script>` ``` Output: ```js var _templateObject, _templateObject2; foo(_templateObject || (_templateObject = babelHelpers.taggedTemplateLiteral(["<\/script>"]))); bar(_templateObject2 || (_templateObject2 = babelHelpers.taggedTemplateLiteral(["<script>", "<\/script>"])), content); ``` Closes #15306
777ea0b to
1b18457
Compare
…ces (#15830) Fixes the issue identified in #15664 where the tagged template transform plugin incorrectly handled template literals containing escape sequences. ### Examples ```js // Input foo`</script>\n` // Before (incorrect): foo(_t || (_t = taggedTemplateLiteral(["</script>\\n"]))); // After (correct): foo(_t || (_t = taggedTemplateLiteral(["</script>\n"], ["</script>\\n"]))); // Input with invalid escape foo`</script>\u` // Before (syntax error): foo(_t || (_t = taggedTemplateLiteral(["</script>\\u"]))); // After (correct): foo(_t || (_t = taggedTemplateLiteral([void 0], ["</script>\\u"])));
…ces (#15830) Fixes the issue identified in #15664 where the tagged template transform plugin incorrectly handled template literals containing escape sequences. ### Examples ```js // Input foo`</script>\n` // Before (incorrect): foo(_t || (_t = taggedTemplateLiteral(["</script>\\n"]))); // After (correct): foo(_t || (_t = taggedTemplateLiteral(["</script>\n"], ["</script>\\n"]))); // Input with invalid escape foo`</script>\u` // Before (syntax error): foo(_t || (_t = taggedTemplateLiteral(["</script>\\u"]))); // After (correct): foo(_t || (_t = taggedTemplateLiteral([void 0], ["</script>\\u"])));

Implement a transformation plugin for tagged template expressions containing </script> tags to prevent script tag issues in browser environments.
This plugin transforms tagged template literals containing
</script(case-insensitive) to use a helper function call with template caching, matching esbuild's behavior.Example
Transforms tagged templates to use cached template objects with a helper function:
Input:
Output:
Closes #15306