fix: escape all special characters in identifiers when stringifying#1802
fix: escape all special characters in identifiers when stringifying#1802spokodev wants to merge 3 commits into
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesEscape character expansion
Estimated code review effort: 1 (Trivial) | ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/stringify.spec.ts`:
- Around line 31-35: The test data array starting at line 31 is missing coverage
for the backtick character and specific round-trip selector contexts. Add test
cases to this array for the backtick escape (backtick paired with its escaped
equivalent string), and add three additional test cases for the exact round-trip
selector contexts mentioned in the PR: the class selector with escaped ampersand
(`.x\&y`), the data attribute selector with escaped equals (` [data-foo\=bar]`),
and the class selector with escaped braces (`.a\{b\}c`). Each test case should
follow the existing pattern of providing the input string, the expected escaped
output using String.raw, and a descriptive label.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 902c7b00-7785-47ad-87c4-7401897ac6e0
📒 Files selected for processing (2)
src/stringify.spec.tssrc/stringify.ts
c24aff0 to
f6438f1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/stringify.spec.ts`:
- Around line 62-66: The `it.each(selectors)` test currently nests
`parse(stringify(parse(selector)))` too deeply and violates the
`unicorn/max-nested-calls` rule. In `stringify.spec.ts`, split the chained
`parse`/`stringify`/`parse` flow into intermediate variables inside the test
callback, then compare the final parsed value against the original
`parse(selector)` result to keep the assertion behavior unchanged while reducing
nesting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 403712ba-b24e-454c-8eae-7d600752d7c3
📒 Files selected for processing (2)
src/stringify.spec.tssrc/stringify.ts
`charsToEscapeInName` was missing several characters that are special in
selector syntax, so `stringify()` emitted selectors that re-parse to a
different AST or fail to parse. This breaks round-tripping of modern
class names (arbitrary values, CSS nesting), e.g.:
stringify(parse(".x\\&y")) // => ".x&y" (two selectors)
stringify(parse("[data-foo\\=x]")) // => "[data-foo=x]" (name+operator)
stringify(parse(".a\\{b\\}c")) // => ".a{b}c"
Add `=`, `&`, `?`, `@`, `` ` ``, `{` and `}` so every special ASCII
character in an identifier is escaped, matching the existing handling of
`# > < / , ;` etc.
f6438f1 to
b4e3f20
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/stringify.spec.ts`:
- Around line 53-67: The stringify round-trip test in Stringify round-trips
special characters in identifiers is missing coverage for the newly escaped @
character. Update the selectors array in the describe block to include an
`@-containing` selector (for example one using parse and stringify with a
data-attribute style selector), so the round-trip assertion exercises the same
code paths as the existing cases for &, =, {}, /, and ?.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f3b18178-8cec-4b37-ac03-5657f61e1d41
📒 Files selected for processing (2)
src/stringify.spec.tssrc/stringify.ts
Bug
charsToEscapeInNameis missing several characters that are special in selector syntax, sostringify()produces selectors that re-parse to a different AST (or fail to parse).parse(stringify(x))is no longer equal tox:This breaks round-tripping of modern class names that contain these characters — e.g. Tailwind arbitrary values and CSS-nesting fragments (
grid-cols-[&],data-[x=y], …).Fix
Add
=,&,?,@,`,{and}tocharsToEscapeInName, so every special ASCII character in an identifier is escaped — completing the set alongside the# > < / , ;etc. that are already handled. Escaping is transparent toparse, so existing round-trips are unaffected; only previously-corrupted ones are fixed.Tests
Added cases to the "Stringify CSS spec compliance" block in
stringify.spec.ts(a=b→a\=b,&,{},?,@). They fail onmain(the characters are emitted unescaped) and pass with this change. Full suite: 860 passing, no regressions.Summary by CodeRabbit
Bug Fixes
=,&,?,@, backticks,{, and}to generate valid selector output.Tests