Refactor block parsing and improve whitespace handling - #278
Merged
Conversation
…s, dedupe parser internals Two real bugs surfaced while reviewing the parser for duplication: - Lexer.skip_whitespace_in_range() used char-types.ts's narrower is_whitespace() (space/tab only, no newlines), while tokenize.ts's own hot-path whitespace skip correctly includes newlines. Any at-rule prelude or nth-expression relying on skip_whitespace_in_range() to jump straight to the next real token (AtRulePreludeParser, ANplusBParser) could silently split a media/supports query in two when a newline fell between components, e.g. "@media screen\nand (min-width: 768px)" produced two MediaQuery nodes instead of one. Fixed by checking CHAR_WHITESPACE | CHAR_NEWLINE directly, matching the rest of the file; removed the now-dead char-types.ts is_whitespace. - FLAG_HAS_DECLARATIONS was only ever set on STYLE_RULE nodes, even though the public Atrule type promises `has_declarations: boolean`. At-rules with direct declarations (@font-face, @page, or a nested @media under CSS Nesting) always reported has_declarations: false. Also removed duplication that had built up across the parser: - parse.ts: style rules and at-rule bodies each had their own ~35-line copy of the "declaration, nested rule, or nested at-rule" loop; extracted into parse_block_children() (which also carries the has_declarations fix to at-rules). - parse-atrule-prelude.ts: the "scan to matching closing paren" loop was copy-pasted across 7 call sites (function conditions, media/supports features, @import's url()/layer()/supports(), @scope); extracted into scan_matching_paren(). - parse-selector.ts: skip_whitespace() had its own from-scratch copy of the whitespace/comment skip that Lexer.skip_whitespace_in_range() already implements (correctly) elsewhere; now delegates to it. Merged create_node/create_node_at into one method with defaulted line/column. - walk.ts: walk()/traverse() each duplicated their recursive helper's body just to avoid one extra node-wrapper allocation for the root node; now delegate directly. - value-node-parser.ts: dropped a one-line-only wrapper. None of tokenize.ts's character-level hot loops (next_token_fast, consume_number, consume_ident_or_function, etc.) were touched — those are deliberately inlined for performance. Verified no regression with a before/after timing comparison parsing bootstrap.css (100 iterations): ~57-58ms/parse on both sides. All 1382 existing tests plus 4 new regression tests pass; typecheck, lint, and knip are clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017XjfDK7or6VrnJ41vBhxrK
Contributor
🎉 Package Size Decrease
|
Member
Author
|
CI is green except Audit packages, which is unrelated to this PR: this branch doesn't touch All other checks (unit tests, types, lint, knip, dependency diff, pack) pass. Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR refactors the CSS parser to eliminate code duplication in block parsing logic and improves whitespace handling in at-rule prelude parsing. The changes consolidate three separate block-parsing implementations into a single reusable method and fix a regression where newlines in at-rule prelude tokens could cause incorrect AST structure.
Key Changes
Extract
parse_block_children()method: Consolidated duplicated block parsing logic fromparse_style_rule()andparse_atrule()into a single private method that handles declarations, nested rules, and at-rules with adeclarations_onlyparameter to control parsing behavior.Fix whitespace handling in at-rule prelude parsing: Introduced
scan_matching_paren()helper method to consistently handle parenthesis matching across multiple at-rule prelude parsing contexts (media features, supports queries, layer functions, etc.), fixing a regression where newlines between tokens could desync the whitespace-skip helper from the tokenizer's newline handling.Simplify declaration parsing logic: Refactored
parse_declaration()to use clearer control flow by consolidating token type checks and reducing nested conditionals.Improve selector parser: Consolidated
create_node()andcreate_node_at()methods into a singlecreate_node()with optional line/column parameters that default to the current token position, reducing code duplication.Simplify walk/traverse functions: Refactored
walk()andtraverse()inwalk.tsto delegate to their internal_walk()and_traverse()implementations, eliminating duplicated logic.Remove unused code: Removed unused
is_whitespace()export fromchar-types.tsand unusedcreate_operator_node()fromvalue-node-parser.ts.Add regression tests: Added test cases for newline handling in
@mediaqueries and@supportsconditions, and expandedhas_declarationsproperty tests to cover at-rules with direct declarations.Implementation Details
The
parse_block_children()method accepts anownernode anddeclarations_onlyflag, properly settingFLAG_HAS_DECLARATIONSon the owner when direct declaration children are found. This flag is now correctly set for both style rules and at-rules (including nested conditional at-rules under CSS Nesting).The
scan_matching_paren()helper returns a tuple of[content_end, close_end, matched]to provide consistent parenthesis matching across all at-rule prelude contexts, with sensible fallback values when EOF is encountered before a matching closing paren.https://claude.ai/code/session_017XjfDK7or6VrnJ41vBhxrK