feat(grit): add JSX node name mappings for GritQL patterns#7285
feat(grit): add JSX node name mappings for GritQL patterns#7285arendjr merged 2 commits intobiomejs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: e107dc9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe PR adds JSX support to JsTargetLanguage by mapping JSX node names to new internal kinds, providing reverse name_for_kind entries, and defining named_slots_for_kind for JSX constructs (elements, self-closing/opening/closing elements, attributes, expression children, text, namespace names). It adds a TSX test fixture and a Grit spec exercising these JSX kinds, and a changeset entry noting partial fix for #6782. No public APIs were changed. Assessment against linked issues
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
rriski
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit (1)
1-7: Broaden the spec to exercise all newly added JSX node kindsGood starter test. To cover the mappings you’ve added, please include the remaining JSX kinds so we don’t regress unnoticed.
Apply this diff to expand coverage:
or { jsx_expression(), - jsx_attribute() + jsx_attribute(), + jsx_element(), + jsx_self_closing_element(), + jsx_opening_element(), + jsx_closing_element() }Follow-up: once the fields are finalised, add minimal where-clauses to assert the expected fields exist (e.g. children/name/attributes).
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.tsx (1)
1-4: Add samples that hit self-closing elements and React.Fragment namesThis will exercise the extra mappings (self-closing/opening/closing) and the member-style tag name used in issue #6782.
<div className="test" id={someId}> {content} {showHeader && <h1>Title</h1>} + <img alt="logo" src={logoSrc} /> + <React.Fragment> + <span>Inside</span> + </React.Fragment> </div>crates/biome_grit_patterns/src/grit_target_language/js_target_language.rs (1)
48-53: Add missing JSX kinds for Tree‑sitter parityConfirmed the repo exposes these JsSyntaxKind variants (e.g. JSX_TEXT, JSX_FRAGMENT, JSX_MEMBER_NAME, JSX_NAMESPACE_NAME, JSX_SPREAD_ATTRIBUTE). Please add mappings (and mirror them in kind_by_name, name_for_kind and any named_slots_for_kind) in crates/biome_grit_patterns/src/grit_target_language/js_target_language.rs near the existing JSX block:
- "jsx_text" => JSX_TEXT
- "jsx_fragment" => JSX_FRAGMENT
- "jsx_member_expression" => JSX_MEMBER_NAME (also add "jsx_member_name" => JSX_MEMBER_NAME for robustness)
- "jsx_namespaced_name" => JSX_NAMESPACE_NAME (also add "jsx_namespace_name" => JSX_NAMESPACE_NAME)
- "jsx_spread_attribute" => JSX_SPREAD_ATTRIBUTE
Suggested snippet to append/update:
"jsx_expression" => JSX_EXPRESSION_CHILD, "jsx_attribute" => JSX_ATTRIBUTE, "jsx_element" => JSX_ELEMENT, "jsx_self_closing_element" => JSX_SELF_CLOSING_ELEMENT, "jsx_opening_element" => JSX_OPENING_ELEMENT, "jsx_closing_element" => JSX_CLOSING_ELEMENT, "jsx_text" => JSX_TEXT, "jsx_fragment" => JSX_FRAGMENT, "jsx_member_expression" => JSX_MEMBER_NAME, "jsx_member_name" => JSX_MEMBER_NAME, "jsx_namespaced_name" => JSX_NAMESPACE_NAME, "jsx_namespace_name" => JSX_NAMESPACE_NAME, "jsx_spread_attribute" => JSX_SPREAD_ATTRIBUTE,This is optional refactor to avoid future mismatches with Tree‑sitter node names — do it when convenient and run the grit mappings tests.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_grit_patterns/src/grit_target_language/js_target_language.rs(3 hunks)crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit(1 hunks)crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.tsxcrates/biome_grit_patterns/src/grit_target_language/js_target_language.rscrates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.tsxcrates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_grit_patterns/src/grit_target_language/js_target_language.rs
🧠 Learnings (5)
📚 Learning: 2025-08-17T08:57:34.751Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:57:34.751Z
Learning: Applies to crates/biome_parser/xtask/codegen/src/*_kinds_src.rs : Add src/<language>_kinds_src.rs under xtask/codegen that returns a static KindSrc
Applied to files:
crates/biome_grit_patterns/src/grit_target_language/js_target_language.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`
Applied to files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_test.rs : Create tests/spec_test.rs implementing the run(spec_input_file, _expected_file, test_directory, _file_type) function as shown and include!("language.rs")
Applied to files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/*.jsonc : `.jsonc` snapshot files must contain an array of code strings; they are interpreted in script mode (no ESM syntax)
Applied to files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_tests.rs : Create tests/spec_tests.rs in the biome_html_formatter crate that generates tests via tests_macros::gen_tests! for all HTML files at tests/specs/html/**/*.html
Applied to files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
🔇 Additional comments (2)
crates/biome_grit_patterns/src/grit_target_language/js_target_language.rs (2)
78-83: LGTM: reverse mappings are symmetricalThe name_for_kind entries correctly mirror kind_by_name for the new JSX kinds.
106-116: Incorrect field-name suggestion for JSX_ATTRIBUTE mappingThe premise to rename
"initializer"to"value"is mistaken. In the JavaScript grammar’snode-types.json:
- The
jsx_attributeandjsx_expressionnode types themselves have no named fields.- Only the opening/self-closing JSX element nodes expose a single field named
"attribute", not"attributes"or"value".Neither
"initializer"nor"value"appear as field names; the current mapping doesn’t align with the grammar’s field definitions. You can safely ignore the proposed change to"value".Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit (2)
4-7: Exercise all newly mapped JSX kinds in this specYou added mappings for six JSX kinds, but this spec only hits two. Expanding the OR set will catch regressions early.
Apply this diff to broaden coverage (ensure the TSX fixture contains examples for each, e.g. add a self-closing element):
or { jsx_expression(), - jsx_attribute() + jsx_attribute(), + jsx_element(), + jsx_self_closing_element(), + jsx_opening_element(), + jsx_closing_element() }
4-7: Add a companion spec to assert named slot accessSince this PR also wires named_slots_for_kind, add a second spec that verifies at least one slot per kind (e.g. attribute.name, attribute.initializer, opening_element.name, element.children, expression.expression). This will prevent silent breakage if slot indices drift.
Happy to draft a follow-up spec that targets slot access against the same TSX fixture—shout if you want me to propose concrete patterns.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_grit_patterns/src/grit_target_language/js_target_language.rs(3 hunks)crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit(1 hunks)crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_grit_patterns/src/grit_target_language/js_target_language.rs
- crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.tsx
🧰 Additional context used
📓 Path-based instructions (2)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
🧠 Learnings (4)
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`
Applied to files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/*.jsonc : `.jsonc` snapshot files must contain an array of code strings; they are interpreted in script mode (no ESM syntax)
Applied to files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_test.rs : Create tests/spec_test.rs implementing the run(spec_input_file, _expected_file, test_directory, _file_type) function as shown and include!("language.rs")
Applied to files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_tests.rs : Create tests/spec_tests.rs in the biome_html_formatter crate that generates tests via tests_macros::gen_tests! for all HTML files at tests/specs/html/**/*.html
Applied to files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
🔇 Additional comments (2)
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit (2)
1-2: Directives look goodThe engine and language headers are correct for a basic JSX smoke test.
2-2: Language js(jsx) is appropriate for jsx_nodes.tsxNo TS-specific syntax was found in the
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.tsxfixture, so keepinglanguage js(jsx)is correct.
arendjr
left a comment
arendjr
left a comment
There was a problem hiding this comment.
Thanks, that’s helpful! Could you add a short changeset?
Thanks for a swift review! Added changesets and implemented coderabbit suggestions. |
Add JSX TreeSitter node mappings to JsTargetLanguage for GritQL support - Add jsx_expression, jsx_attribute, jsx_element, jsx_self_closing_element, jsx_opening_element, jsx_closing_element, jsx_text, jsx_namespace_name mappings to JsTargetLanguage Partially resolves biomejs#6782
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit (2)
5-12: Add a slots-focused assertion to prevent regressionsThis OR-pattern proves kind recognition but doesn’t assert slot wiring (e.g., attribute.name, opening_element.name, expression.value). Consider a companion spec that binds fields in a where clause and logs them (either via named parameters like
jsx_attribute(name: $name)or by binding the node and accessing its fields, depending on GritQL syntax).
11-11: Mind potential flakiness from jsx_text()
jsx_text()may match whitespace-only text nodes depending on the fixture’s formatting. If snapshots prove noisy, trim the fixture’s inter-element whitespace or filter to non-empty text in the spec.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (4)
.changeset/cyan-jars-shake.md(1 hunks)crates/biome_grit_patterns/src/grit_target_language/js_target_language.rs(3 hunks)crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit(1 hunks)crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/cyan-jars-shake.md
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.tsx
- crates/biome_grit_patterns/src/grit_target_language/js_target_language.rs
🧰 Additional context used
📓 Path-based instructions (2)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
🧠 Learnings (3)
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`
Applied to files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_test.rs : Create tests/spec_test.rs implementing the run(spec_input_file, _expected_file, test_directory, _file_type) function as shown and include!("language.rs")
Applied to files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_tests.rs : Create tests/spec_tests.rs in the biome_html_formatter crate that generates tests via tests_macros::gen_tests! for all HTML files at tests/specs/html/**/*.html
Applied to files:
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit
🔇 Additional comments (2)
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.grit (2)
4-13: Nice breadth: this exercises all newly mapped JSX kindsEnumerating expression/attribute/element variants, text, and namespaced names will give good coverage for the mapping layer.
1-2: Please verify the TSX fixture runs underlanguage js(jsx)I located the companion file at
crates/biome_grit_patterns/tests/specs/tsx/jsx_nodes.tsxThe
.gritspec still useslanguage js(jsx). If our test harness requires a stricter TSX pairing, you’ll want to confirm this spec actually executes (and passes) as-is, or switch to a TSX-specific directive.
CodSpeed Performance ReportMerging #7285 will not alter performanceComparing Summary
Footnotes |
Co-authored-by: Arend van Beelen jr. <arend@arendjr.nl>
2 participants
Add JSX TreeSitter node mappings to JsTargetLanguage for GritQL support
jsx_opening_element, jsx_closing_element, jsx_text, jsx_namespace_name mappings to JsTargetLanguage
Partially resolves #6782