fix(compiler): escape backslashes before other escape sequences in styles#6773
fix(compiler): escape backslashes before other escape sequences in styles#6773Arul1998 wants to merge 1 commit into
Conversation
…yles Escaping backslashes last re-escaped the backslash added for backticks, leaving an unescaped backtick that terminated the generated template literal and failed the build. Escape backslashes first, and also escape dollar-brace so CSS content cannot be evaluated as a template literal interpolation. Fixes stenciljs#6710
johnjenkins
left a comment
There was a problem hiding this comment.
Looks good 🙏
The fix itself looks correct - I traced through the escape-order logic (including the \f/\b/\v/\0 branches); moving backslash-escaping to the front, combined with switching those four control-char replacements to double-backslash output, cancels out the reordering so control-char output stays byte-for-byte identical to before. The ${ escape is also safe against pre-existing backslashes in the source.
One test gap: none of the tests (old or new) exercise the \u000c / \u0008 / \u000b / \0 regex branches with an actual raw control-character byte. The new "preserves CSS escape sequences like icon font content" test uses the JS string '\\f101', which is a literal backslash character followed by the text f101 - not a real U+000C form-feed byte.
Suggest adding one more test with a genuine raw control-character byte (e.g. an actual \u000c in input, not its escaped text form) asserted via the same evaluateEsmOutput round-trip pattern used for the other cases - tysm!
Escaping backslashes last re-escaped the backslash added for backticks, leaving an unescaped backtick that terminated the generated template literal and failed the build. Escape backslashes first, and also escape dollar-brace so CSS content cannot be evaluated as a template literal interpolation.
Fixes #6710
What is the current behavior?
GitHub Issue Number: #6710
A component stylesheet containing a backtick, e.g.
fails the whole build with:
When embedding CSS into the generated JS template literal,
css-to-esm.tsescaped backslashes last, which re-escaped the backslash it had just added for backticks (\`became\\`), leaving an unescaped backtick that terminates the template literal early. This is a regression — it worked in 4.29.3, before the escape chain was introduced.${in CSS content had the same class of problem: it was not escaped at all, so it was evaluated as a template literal interpolation at runtime.What is the new behavior?
Backslashes are escaped first, so the escape sequences added afterwards are not re-escaped, and
${is now escaped as well. CSS containing backticks or${compiles and round-trips correctly.Behavior for everything else is unchanged, including the intentional handling of control characters (
\f,\b,\v,\0) and icon-font content likecontent: "\f101"— the generated source for those inputs is identical to before this change.Documentation
N/A — internal compiler fix, no API or documentation changes.
Does this introduce a breaking change?
Testing
escapes backticks in CSS contentunit test: it previously only did a substring check that the broken output also satisfied; it now evaluates the generated module to prove it is valid JavaScript and round-trips the input CSS.${}in CSS content and for icon-font escape sequences (\f101) round-tripping.src/compiler/style/test/css-to-esm.spec.ts: 31/31 pass locally; full jest suite run locally as well.Before this change, the same input reproduced the Rollup parse error from the issue.
Other information
N/A