Conversation
👷 Deploy Preview for biomejs processing.
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
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:
WalkthroughThis PR adds a new documentation page "GritQL Plugin Recipes" and registers it in the site navigation (Guides sidebar and Recipes list) with a "new" badge. It extends the playground to persist and control a pane state: introduces pane values (Diagnostics, Console, GritQL), adds Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/docs/recipes/gritql-plugins.mdx`:
- Around line 23-29: The intro copy contains two typos: replace "plyaground"
with "playground" and "langauge" with "language" in the text near the sentence
"Alternatively, you can navigate the plyaground link attached to each example."
and the heading "Below, there's a collection of examples for
JavaScript/TypeScript langauge." so the copy reads "playground" and "language".
- Line 567: The Markdown link text "Try this example in the Playground"
currently wraps the URL with extra angle brackets—specifically the URL that
begins with "/playground?tab=syntax..."—which breaks rendering; fix it by
removing the surrounding "<" and ">" so the link becomes [Try this example in
the
Playground](/playground?tab=syntax&pane=GritQL&code=...&gritQuery=...&language=css&gritTargetLanguage=CSS),
keeping the full encoded query string intact.
- Around line 576-606: The Playground link currently encodes a query that only
searches for `` `float: $value` `` so it won't reproduce the documented `clear`
match; update the link's gritQuery parameter (the encoded query in the "[Try
this example in the Playground]" URL) to a query that includes both `` `float:
$value` `` and `` `clear: $value` `` (or the full rule from the grit snippet
that uses $property <: or { `float`, `clear` }) so clicking the Playground
reproduces matches for both the `float: left` and `clear: both` examples shown.
In `@src/playground/components/DiagnosticsPane.tsx`:
- Around line 42-56: Remove the local state mirror created by
useState(currentPane) and make the Tabs component controlled by the prop
currentPane; delete pane and setPane and stop calling setPane in onSelect.
Change onSelect to only call setPlaygroundState(state => ({ ...state, pane: tab
as PlaygroundPane })) so the parent/URL-driven playgroundState.pane drives the
selected tab. Update any references to pane to use currentPane and remove the
unused on-state handlers (pane, setPane, useState) from DiagnosticsPane.
In `@src/playground/PlaygroundLoader.tsx`:
- Line 552: Remove the debug console.log that prints the entire playground
state; specifically delete the console.log("here", playgroundState) call in
PlaygroundLoader.tsx (or replace it with a safe, opt-in debug mechanism such as
a logger gated by a debug flag if you need retained diagnostics). Ensure no
other code paths reference that exact console.log and that sensitive data
(playgroundState) is not logged in production.
- Around line 420-422: The pane value from the URL is being blindly cast to
PlaygroundState["pane"]; instead, validate the string returned by
searchParams.get("pane") against the known allowed pane values and fall back to
defaultPlaygroundState.pane on mismatch. In PlaygroundLoader.tsx, replace the
direct cast with a check (e.g., compare against an array/enum of allowed pane
names or use a helper isValidPane) and only use the URL value if it passes
validation—this will prevent invalid values from reaching DiagnosticsPane and
ensure a safe fallback to defaultPlaygroundState.pane.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d51e2a6e-70f9-4e75-bf78-dc8a3b266e6c
📒 Files selected for processing (6)
astro.config.tssrc/content/docs/recipes/gritql-plugins.mdxsrc/playground/Playground.tsxsrc/playground/PlaygroundLoader.tsxsrc/playground/components/DiagnosticsPane.tsxsrc/playground/types.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/content/docs/recipes/gritql-plugins.mdx (1)
567-567:⚠️ Potential issue | 🟡 MinorRemove the stray angle brackets from this link.
Line 567 wraps the href in
<...>, which breaks the Markdown link instead of rendering a clickable Playground link.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/recipes/gritql-plugins.mdx` at line 567, The Markdown link text "Try this example in the Playground" currently wraps the URL in stray angle brackets (<...>) which prevents the link from rendering; edit src/content/docs/recipes/gritql-plugins.mdx to remove the surrounding '<' and '>' so the link becomes [Try this example in the Playground](<URL>) without extra angle brackets around the entire href, ensuring the Playground URL is a normal Markdown link.
🧹 Nitpick comments (1)
src/playground/components/DiagnosticsPane.tsx (1)
49-81: Keep persisted pane ids separate from tab labels.These
keyvalues now backPlaygroundState.paneand the URL state, so using visible copy as the identifier makes future label tweaks a breaking change for deep links. I’d reuse the shared pane constants/internal ids forkey, and keep the capitalised text intitleonly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/playground/components/DiagnosticsPane.tsx` around lines 49 - 81, The tab objects in DiagnosticsPane (the array passed to tabs) use visible labels as their key ("Diagnostics", "Console", "GritQL"), but those keys back PlaygroundState.pane and URL state; change each object's key to the shared internal pane identifier constant (e.g., PaneId.Diagnostics, PaneId.Console, PaneId.GritQL or whatever shared constants your app uses) while leaving the human-facing string in title and keeping children as-is (affects DiagnosticsListTab, DiagnosticsConsoleTab, GritQLSearchTab usage). Ensure PlaygroundState.pane and any URL serialization use these constants so renaming titles won't break deep links.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/docs/recipes/gritql-plugins.mdx`:
- Line 489: Update the Playground link so it encodes the generic rule
``$property: $value !important`` instead of the specific ``color: $value
!important``—regenerate the URL used in the markdown link (the encoded code
param and the gritQuery parameter) to include the generic rule string so both
`margin: 0 !important` and `color: red !important` are matched as described in
the prose.
- Line 23: The sentence "Alternatively, you can navigate the playground link
attached to each example." incorrectly promises a playground link for every
example; either add playground links to the examples that are missing them or
change this sentence to a softer phrase such as "a playground link is available
for some examples" or "where available, you can navigate the playground link
attached to examples" so it no longer guarantees a link for every example;
update the line text in the docs file (the sentence currently at the start of
the examples section) and scan the subsequent example headings to ensure wording
matches the actual availability of playground links.
---
Duplicate comments:
In `@src/content/docs/recipes/gritql-plugins.mdx`:
- Line 567: The Markdown link text "Try this example in the Playground"
currently wraps the URL in stray angle brackets (<...>) which prevents the link
from rendering; edit src/content/docs/recipes/gritql-plugins.mdx to remove the
surrounding '<' and '>' so the link becomes [Try this example in the
Playground](<URL>) without extra angle brackets around the entire href, ensuring
the Playground URL is a normal Markdown link.
---
Nitpick comments:
In `@src/playground/components/DiagnosticsPane.tsx`:
- Around line 49-81: The tab objects in DiagnosticsPane (the array passed to
tabs) use visible labels as their key ("Diagnostics", "Console", "GritQL"), but
those keys back PlaygroundState.pane and URL state; change each object's key to
the shared internal pane identifier constant (e.g., PaneId.Diagnostics,
PaneId.Console, PaneId.GritQL or whatever shared constants your app uses) while
leaving the human-facing string in title and keeping children as-is (affects
DiagnosticsListTab, DiagnosticsConsoleTab, GritQLSearchTab usage). Ensure
PlaygroundState.pane and any URL serialization use these constants so renaming
titles won't break deep links.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 910c4fc0-45e6-4502-bff3-73164444d5ee
📒 Files selected for processing (3)
src/content/docs/recipes/gritql-plugins.mdxsrc/playground/PlaygroundLoader.tsxsrc/playground/components/DiagnosticsPane.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/playground/PlaygroundLoader.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/content/docs/recipes/gritql-plugins.mdx (1)
23-23:⚠️ Potential issue | 🟡 MinorSoften the promise about playground links.
This line still states "the playground link attached to each example", but a few examples (e.g., the expanded import/require pattern at lines 143-154 and the extended hardcoded colours regex at lines 534-549) don't have one. Consider "a playground link is available for most examples" or similar.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/recipes/gritql-plugins.mdx` at line 23, Update the sentence "Alternatively, you can navigate the playground link attached to each example." to soften the claim; replace it with wording such as "Alternatively, a playground link is available for most examples." or "Alternatively, many examples include a playground link." Ensure you update the single occurrence in the document (the line containing that exact sentence in gritql-plugins.mdx) so it no longer implies every example has a playground link.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/docs/recipes/gritql-plugins.mdx`:
- Around line 360-361: Update the explanatory sentence to reference the correct
line numbers: change "lines 1-3" to "lines 1, 2, and 5" (the matched `any`
annotations per the ins markers) and change the mention of `unknown` being on
line 4 to state it is on line 6; adjust the sentence text in the
gritql-plugins.mdx content accordingly so the line-number claims match the ins
markers.
- Line 293: The Playground links wrapped in angle brackets (e.g., the link text
"Try this example in the Playground") should be updated to standard Markdown
link syntax by removing the surrounding "<" and ">" so the href is written as
(https://...query...) instead of (</...>); make the same change for the other
instances referenced (the links at the lines containing the second and third
"Try this example in the Playground" usages near lines 571 and 690) so all three
Playground links use plain parentheses without angle brackets.
---
Duplicate comments:
In `@src/content/docs/recipes/gritql-plugins.mdx`:
- Line 23: Update the sentence "Alternatively, you can navigate the playground
link attached to each example." to soften the claim; replace it with wording
such as "Alternatively, a playground link is available for most examples." or
"Alternatively, many examples include a playground link." Ensure you update the
single occurrence in the document (the line containing that exact sentence in
gritql-plugins.mdx) so it no longer implies every example has a playground link.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c8a86a36-fcf8-4fe4-bda7-907b95bc4eea
📒 Files selected for processing (1)
src/content/docs/recipes/gritql-plugins.mdx
morgante
left a comment
There was a problem hiding this comment.
Many of the playground links I clicked didn't actually work/parse, which I think would make things more confusing rather than less.
I'd start with just the playground improvement + a few targeted recipes.
Once that's working I think it might make sense to just import most of https://docs.grit.io/ over once there's a validation loop for the queries.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pnpm-workspace.yaml`:
- Line 1: Fix the typo in the workspace config by changing the value of the YAML
key linkWorkspacePackages from the incorrect token "tru" to the boolean true so
pnpm recognizes it; locate the linkWorkspacePackages entry in the
pnpm-workspace.yaml and replace the incorrect value with true (unquoted) to
ensure workspace packages are linked.
In `@src/content/docs/recipes/gritql-plugins.mdx`:
- Line 336: The Playground link currently points to a different rule (`try { $_
} catch ($_) {}`) instead of the CST example discussed; update the URL's
gritQuery parameter so it opens the CST pattern shown in the docs (e.g., the
JsCatchClause example: JsCatchClause(body = JsBlockStatement(statements = [])))
or change the explanatory text to match the encoded query; locate the link near
the example and replace the encoded query payload accordingly so clicking the
Playground runs the exact CST example being explained.
- Around line 372-389: The rule currently flags every JsVariableStatement()
whose text starts with "let" without checking for reassignment; update the
matcher to inspect the variable declarators in the matched $stmt (e.g., iterate
declarators/binding names from $stmt) and only call register_diagnostic when
none of those bindings are reassigned or updated elsewhere (detect
AssignmentExpression, UpdateExpression, or any write to the binding identifier);
keep using JsVariableStatement() and $stmt but add the additional check for
reassignments before emitting the diagnostic so the rule truly implements
"prefer const" rather than banning let unconditionally.
- Around line 274-285: The regex r".*,.*,.*,.*" on JsParameters()
false-positives for commas inside default values; instead perform a
structure-aware parameter count check: change the where clause on JsParameters()
to inspect the parsed parameter nodes (e.g., use a length/count check on the
$params node such as count($params) >= 4 or $params.len >= 4 depending on the
DSL) and keep the register_diagnostic(...) call as-is to report when the
parameter node count is 4 or more; reference JsParameters(), $params, and
register_diagnostic when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 820835e6-a73d-495a-831c-f1bd3da9ea8e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
pnpm-workspace.yamlscripts/package.jsonscripts/test-gritql.jssrc/content/docs/recipes/gritql-plugins.mdx
| `NodeName()` for any instance, or `NodeName(field = ...)` to match specific | ||
| children. | ||
|
|
||
| ### Common node names |
There was a problem hiding this comment.
These are extremely helpful! Thank you!
There was a problem hiding this comment.
♻️ Duplicate comments (5)
src/content/docs/recipes/gritql-plugins.mdx (5)
367-368:⚠️ Potential issue | 🟡 MinorThese line references are still wrong.
In the sample,
anyappears on Lines 1, 2, and 5, andunknownis on Line 6. The current prose says “lines 1-3” and “line 4”, so the explanation doesn’t match the markers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/recipes/gritql-plugins.mdx` around lines 367 - 368, The explanatory text that currently states “Every `any` annotation on lines 1-3 is matched. The `unknown` on line 4 is a different type and is not affected.” is incorrect; update that sentence to match the actual markers so it reads that `any` appears on lines 1, 2, and 5 and `unknown` appears on line 6 (replace the “lines 1-3” and “line 4” phrasing accordingly) so the prose aligns with the sample markers in gritql-plugins.mdx.
372-389:⚠️ Potential issue | 🟠 MajorThis recipe bans
let; it doesn’t really enforce “prefer const”.Lines 382-383 flag any
JsVariableStatement()whose text starts withlet, including variables that are later reassigned. Either rename the recipe/message to a straightletban, or add the extra analysis needed to prove the binding is never written again.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/recipes/gritql-plugins.mdx` around lines 372 - 389, The recipe currently matches JsVariableStatement() as $stmt and flags any statement starting with "let", which bans let rather than enforcing "prefer const"; update the rule to either (A) rename the rule/message to explicitly ban let (change the diagnostic message registered by register_diagnostic to say "Avoid using let; use const or other patterns" and keep the existing JsVariableStatement() as $stmt where { $stmt <: r"let.*", ... }) or (B) add a write-use analysis that proves the binding is never reassigned before registering the diagnostic (resolve the binding referenced by $stmt, check for subsequent write occurrences/new assignments/updates to that identifier in the scope, and only call register_diagnostic when no writes are found) — target the JsVariableStatement(), $stmt binding lookup, and the register_diagnostic invocation to implement either fix.
300-300:⚠️ Potential issue | 🟡 MinorA few Playground links are still wrapped in
<...>.Lines 300, 401, 500, 578, and 664 use
](</playground?...>), which is easy to render oddly in MDX/Markdown. Plain[text](/playground?...)is the safer form.Also applies to: 401-401, 500-500, 578-578, 664-664
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/recipes/gritql-plugins.mdx` at line 300, Replace the playground links that are using the angle-bracket-wrapped path fragment (pattern like "](</playground?...>)") with the plain Markdown/MDX link form "](/playground?...)" in src/content/docs/recipes/gritql-plugins.mdx; locate the instances matching the string "](</playground" (occurring at the reported link occurrences) and remove the surrounding "<" and ">" so the href is not wrapped, ensuring each Playground link renders correctly in MDX.
274-285:⚠️ Potential issue | 🟠 MajorThis parameter-count recipe overcounts commas.
Lines 274-282 treat every comma in the raw
JsParameters()text as a parameter separator, so defaults/destructuring likefn(a = [1, 2], b, c)can be reported as “4+ parameters”. Either make the matcher structure-aware or call out the limitation in the prose; as written, the recipe is stricter than the heading suggests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/recipes/gritql-plugins.mdx` around lines 274 - 285, The current recipe uses JsParameters() as $params and a raw regex r".*,.*,.*,.*" to count commas, which overcounts when commas appear inside default values or destructured arrays/objects; change the matcher to be structure-aware (e.g., match individual parameter nodes instead of the raw parameter text) or, if you can't parse AST nodes, update the prose to document the limitation; specifically, replace the comma-counting regex check on $params with a node-based count of parameter children (or mention the caveat next to the JsParameters() example and $params/register_diagnostic usage) so defaults/destructuring like fn(a = [1, 2], b, c) are not misreported.
336-336:⚠️ Potential issue | 🟡 MinorThe Playground link is showing a different rule.
Line 336 opens
try { $_ } catch ($_) {}, but this section explainsJsCatchClause(body = JsBlockStatement(statements = [])). Readers who click through won’t be exercising the rule they just learned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/recipes/gritql-plugins.mdx` at line 336, The Playground URL currently loads the try/catch snippet (`try { $_ } catch ($_) {}`) while the doc text is demonstrating `JsCatchClause(body = JsBlockStatement(statements = []))`; update the link so the gritQuery (or displayed code) matches the documented rule: either change the page text to use the try/catch example shown in the URL or replace the gritQuery parameter in the Playground link with the encoded example that exercises `JsCatchClause(body = JsBlockStatement(statements = []))`, ensuring the code sample in the link and the documentation (references to JsCatchClause and JsBlockStatement) are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/content/docs/recipes/gritql-plugins.mdx`:
- Around line 367-368: The explanatory text that currently states “Every `any`
annotation on lines 1-3 is matched. The `unknown` on line 4 is a different type
and is not affected.” is incorrect; update that sentence to match the actual
markers so it reads that `any` appears on lines 1, 2, and 5 and `unknown`
appears on line 6 (replace the “lines 1-3” and “line 4” phrasing accordingly) so
the prose aligns with the sample markers in gritql-plugins.mdx.
- Around line 372-389: The recipe currently matches JsVariableStatement() as
$stmt and flags any statement starting with "let", which bans let rather than
enforcing "prefer const"; update the rule to either (A) rename the rule/message
to explicitly ban let (change the diagnostic message registered by
register_diagnostic to say "Avoid using let; use const or other patterns" and
keep the existing JsVariableStatement() as $stmt where { $stmt <: r"let.*", ...
}) or (B) add a write-use analysis that proves the binding is never reassigned
before registering the diagnostic (resolve the binding referenced by $stmt,
check for subsequent write occurrences/new assignments/updates to that
identifier in the scope, and only call register_diagnostic when no writes are
found) — target the JsVariableStatement(), $stmt binding lookup, and the
register_diagnostic invocation to implement either fix.
- Line 300: Replace the playground links that are using the
angle-bracket-wrapped path fragment (pattern like "](</playground?...>)") with
the plain Markdown/MDX link form "](/playground?...)" in
src/content/docs/recipes/gritql-plugins.mdx; locate the instances matching the
string "](</playground" (occurring at the reported link occurrences) and remove
the surrounding "<" and ">" so the href is not wrapped, ensuring each Playground
link renders correctly in MDX.
- Around line 274-285: The current recipe uses JsParameters() as $params and a
raw regex r".*,.*,.*,.*" to count commas, which overcounts when commas appear
inside default values or destructured arrays/objects; change the matcher to be
structure-aware (e.g., match individual parameter nodes instead of the raw
parameter text) or, if you can't parse AST nodes, update the prose to document
the limitation; specifically, replace the comma-counting regex check on $params
with a node-based count of parameter children (or mention the caveat next to the
JsParameters() example and $params/register_diagnostic usage) so
defaults/destructuring like fn(a = [1, 2], b, c) are not misreported.
- Line 336: The Playground URL currently loads the try/catch snippet (`try { $_
} catch ($_) {}`) while the doc text is demonstrating `JsCatchClause(body =
JsBlockStatement(statements = []))`; update the link so the gritQuery (or
displayed code) matches the documented rule: either change the page text to use
the try/catch example shown in the URL or replace the gritQuery parameter in the
Playground link with the encoded example that exercises `JsCatchClause(body =
JsBlockStatement(statements = []))`, ensuring the code sample in the link and
the documentation (references to JsCatchClause and JsBlockStatement) are
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6dc018bb-ce0a-44b4-813e-2a2e2db7dd02
📒 Files selected for processing (1)
src/content/docs/recipes/gritql-plugins.mdx
| | `TsInterfaceDeclaration` | `interface Foo { ... }` | | ||
| | `JsxElement` | `<div>...</div>` | | ||
| | `JsxSelfClosingElement` | `<img />` | | ||
| | `JsxAttribute` | `className="test"`, `disabled` | |
There was a problem hiding this comment.
One gap I've struggled with is knowing whether node name supports arguments or casting. (Sorry I don't know the correct terminology).
For example:
Some functions allow you to bind a variable directly:
JsxOpeningElement(name = $componentName)
Sometimes you can cast the variable:
JsxName() as $name
When using the GritQL playground you will get an error if you don't reference the argument exactly:
JsxOpeningElement(name = $componentName, $test)
Are those arguments described in the js.ungram, or by some other means?
Summary
I've seen multiple social posts from users complaining about the fact that there's a lack of examples on how to use GritQL in Biome.
I'm no expert in the language, so I shamefully used an AI agent to help craft some "real-world" examples, which explain parts of the GritQL syntax, with some code snippets.
I used this skill https://skills.sh/github/awesome-copilot/documentation-writer for the writing, plus I enforced the agent to use the same pattern we use in our docs:
I also added the possibility to save the tab in the URL/playground state, so that now each example is paired with a playground link, which lands on the GritQL + Syntax tabs, which I think are the most important ones in this context.
I reviewed the majority of the examples. The Playground changes were done by me.
cc @morgante and @arendjr for feedback regarding the exapmles