Skip to content

Format at-rule preludes with structured parsing instead of regexes#230

Open
bartveneman wants to merge 1 commit into
mainfrom
claude/atrule-prelude-structured-parsing
Open

Format at-rule preludes with structured parsing instead of regexes#230
bartveneman wants to merge 1 commit into
mainfrom
claude/atrule-prelude-structured-parsing

Conversation

@bartveneman

Copy link
Copy Markdown
Member

Summary

  • format_atrule_prelude reformatted @media/@supports/@container/@layer/@scope/@import preludes with 13 hand-written regexes applied to raw text. This missed cases the regex whitelist didn't cover — e.g. @container STYLE(--foo: bar) never got lowercased, since style wasn't in the hardcoded function-name list — and any new at-rule syntax needs a new regex to work correctly.
  • @projectwallace/css-parser (already used for everything else in this library) ships a real structured parser for this that was previously disabled (parse_atrule_preludes: false). Flipped it on and added a new internal printer, used only by print_atrule, that dispatches on each prelude child's node type instead of pattern-matching text.
  • No public API changes. format_atrule_prelude — the exported partial formatter documented in the README — is completely untouched: same signature, same regex implementation, same tests. It's now also reused internally as the safety-net fallback whenever the structured parser can't represent something losslessly (an at-rule it doesn't recognize, a comment inside a prelude, or the standalone @layer name; statement form).
  • While building this I found and worked around several real bugs in the dependency's structured prelude parser (dotted @layer names getting split at the dot and corrupting cascade semantics if naively reprinted, calc()/env()/other function calls silently dropped from media-feature and @supports condition values, @supports selector(...) returning nothing at all, and a few smaller ones). Full writeup with repro snippets in PARSER_ISSUES.md, since these are worth fixing upstream.
  • Verified via differential testing against the previous implementation across ~170 hand-picked and combinatorial cases (in addition to the existing test suite) to catch regressions the unit tests alone wouldn't — only one divergence found on genuinely invalid CSS input, and two on cases where the old regex output was actually wrong (e.g. @namespace svg url(http://...) — the old regex incorrectly inserted a space inside http://, which the new structured path fixes automatically).

Test plan

  • vitest run — all 269 tests pass (259 existing, unchanged, + 10 new covering the previously-uncovered cases this rework specifically fixes: @container style() lowercasing, function calls surviving in feature/condition values, dotted @layer names, unrecognized at-rules not losing their prelude, nested @supports boolean groups)
  • tsc --noEmit — no type errors
  • oxlint / oxfmt --check — clean
  • Differential fuzz testing against the pre-change implementation (~170 cases) — see PR description above
  • Rough perf sanity check on an at-rule-heavy stylesheet: ~6% slower (structured parsing now happens for every prelude instead of just capturing raw text), not alarming for a correctness fix

Generated by Claude Code

format_atrule_prelude reformatted @media/@supports/@container/@layer/
@scope/@import preludes with 13 hand-written regexes applied to raw
text. This missed real cases the regex whitelist didn't cover - e.g.
@container STYLE(--foo: bar) never got lowercased, since "style"
wasn't in the hardcoded function-name list - and any new syntax
needs a new regex to work correctly.

@projectwallace/css-parser (already used for everything else here)
ships a real structured parser for this that format-css previously
disabled (parse_atrule_preludes: false). Flip it on and add a new
internal printer, used only by print_atrule, that dispatches on each
prelude child's node type instead of pattern-matching text.

format_atrule_prelude itself - the exported partial formatter - is
untouched: same signature, same regex implementation, same tests,
same README example. No public API changes. It's now also reused
internally as the fallback whenever the structured parser can't
represent something losslessly (an at-rule it doesn't recognize, a
comment inside a prelude, or the standalone `@layer name;` statement
form, which has an upstream bug that splits dotted names like
`base.normalize` at the dot - see PARSER_ISSUES.md for this and
several other upstream issues found and worked around while building
this).

Verified via differential testing against the previous implementation
across ~170 hand-picked and combinatorial cases (plus the existing
259-test suite) to catch regressions the unit tests alone wouldn't.
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Duplicate Dependencies (found: 22, threshold: 1)

📦 Package 📋 Versions
@babel/parser
2 versions
  • root@
    • @vitest/coverage-v8@4.1.9
      • magicast@0.5.3
        • @babel/parser@7.29.7

  • root@
    • tsdown@0.22.3
      • rolldown-plugin-dts@0.26.0
        • @babel/generator@8.0.0
          • @babel/parser@8.0.0

@babel/types
2 versions
  • root@
    • @vitest/coverage-v8@4.1.9
      • magicast@0.5.3
        • @babel/parser@7.29.7
          • @babel/types@7.29.7

  • root@
    • tsdown@0.22.3
      • rolldown-plugin-dts@0.26.0
        • @babel/generator@8.0.0
          • @babel/parser@8.0.0
            • @babel/types@8.0.0

@babel/helper-string-parser
2 versions
  • root@
    • @vitest/coverage-v8@4.1.9
      • magicast@0.5.3
        • @babel/parser@7.29.7
          • @babel/types@7.29.7
            • @babel/helper-string-parser@7.29.7

  • root@
    • tsdown@0.22.3
      • ...
        • @babel/parser@8.0.0
          • @babel/types@8.0.0
            • @babel/helper-string-parser@8.0.0

@babel/helper-validator-identifier
2 versions
  • root@
    • @vitest/coverage-v8@4.1.9
      • magicast@0.5.3
        • @babel/parser@7.29.7
          • @babel/types@7.29.7
            • @babel/helper-validator-identifier@7.29.7

  • root@
    • tsdown@0.22.3
      • ...
        • @babel/parser@8.0.0
          • @babel/types@8.0.0
            • @babel/helper-validator-identifier@8.0.2

@rolldown/binding-wasm32-wasi
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-wasm32-wasi@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-wasm32-wasi@1.1.5

picomatch
2 versions
  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • tinyglobby@0.2.17
          • fdir@6.5.0
            • picomatch@4.0.4

  • root@
    • @vitest/coverage-v8@4.1.9
      • vitest@4.1.9
        • @vitest/mocker@4.1.9
          • vite@8.1.4
            • picomatch@4.0.5

rolldown
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • vitest@4.1.9
        • @vitest/mocker@4.1.9
          • vite@8.1.4
            • rolldown@1.1.5

@oxc-project/types
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @oxc-project/types@0.137.0

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @oxc-project/types@0.139.0

@rolldown/binding-android-arm64
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-android-arm64@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-android-arm64@1.1.5

@rolldown/binding-darwin-arm64
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-darwin-arm64@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-darwin-arm64@1.1.5

@rolldown/binding-darwin-x64
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-darwin-x64@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-darwin-x64@1.1.5

@rolldown/binding-freebsd-x64
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-freebsd-x64@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-freebsd-x64@1.1.5

@rolldown/binding-linux-arm-gnueabihf
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-linux-arm-gnueabihf@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-linux-arm-gnueabihf@1.1.5

@rolldown/binding-linux-arm64-gnu
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-linux-arm64-gnu@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-linux-arm64-gnu@1.1.5

@rolldown/binding-linux-arm64-musl
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-linux-arm64-musl@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-linux-arm64-musl@1.1.5

@rolldown/binding-linux-ppc64-gnu
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-linux-ppc64-gnu@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-linux-ppc64-gnu@1.1.5

@rolldown/binding-linux-s390x-gnu
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-linux-s390x-gnu@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-linux-s390x-gnu@1.1.5

@rolldown/binding-linux-x64-gnu
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-linux-x64-gnu@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-linux-x64-gnu@1.1.5

@rolldown/binding-linux-x64-musl
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-linux-x64-musl@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-linux-x64-musl@1.1.5

@rolldown/binding-openharmony-arm64
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-openharmony-arm64@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-openharmony-arm64@1.1.5

@rolldown/binding-win32-arm64-msvc
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-win32-arm64-msvc@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-win32-arm64-msvc@1.1.5

@rolldown/binding-win32-x64-msvc
2 versions
  • root@
    • tsdown@0.22.3
      • rolldown@1.1.3
        • @rolldown/binding-win32-x64-msvc@1.1.3

  • root@
    • @vitest/coverage-v8@4.1.9
      • ...
        • vite@8.1.4
          • rolldown@1.1.5
            • @rolldown/binding-win32-x64-msvc@1.1.5

💡 To find out what depends on a specific package, run: pnpm -r why example-package

⚠️ Package Size Increase

📦 Package 📏 Base Size 📏 Source Size 📈 Size Change
@projectwallace/format-css 8.6 kB 11.8 kB +3.2 kB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants