feat(playground): search with grit in the playground#3895
Conversation
❌ Deploy Preview for biomejs failed. Why did it fail? →
|
WalkthroughThis pull request integrates GritQL query functionality into the playground. Changes span the editor (CodeMirror.tsx with decoration-based highlighting), state management (types.ts, PlaygroundLoader.tsx), worker communication (biomeWorker.ts), and UI components (Playground.tsx, DiagnosticsPane.tsx, new GritQLSearchTab.tsx). A new utility function handles byte-to-code-unit offset conversion. Styling rules are added via _gritql.css. The implementation passes query matches through the component hierarchy, syncs state across editor and diagnostics pane, and enables interactive match navigation. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/playground/CodeMirror.tsx`:
- Around line 133-135: The code currently only returns gritQueryMatchesField
when gritQueryMatches.length > 0, so when gritQueryMatches is defined but empty
the DecorationField (gritQueryMatchesField) is omitted and cannot later receive
annotations; change the condition to include gritQueryMatchesField whenever
gritQueryMatches is defined (i.e., gritQueryMatches !== undefined or truthy but
allow empty array) so the returned extensions are [gritQueryMatchesField,
...baseExtensions] whenever gritQueryMatches exists, ensuring the field is
present to accept future annotations without needing a full rebuild; update the
branch that builds the extensions around gritQueryMatches, referencing
gritQueryMatches, gritQueryMatchesField, and baseExtensions.
- Around line 21-36: buildDecorations currently pushes Decoration objects in the
incoming match order which can be unsorted and causes Decoration.set to throw;
update buildDecorations to collect typed Decoration objects (e.g., Decoration[]
instead of any[]) and sort them by their numeric start position before calling
Decoration.set: for each match call spanInBytesToSpanInCodeUnits([from,to], doc)
to get codeUnitFrom/codeUnitTo, create the decoration via
yellowHighlight.range(codeUnitFrom, codeUnitTo), push to an array typed as
Decoration[], then sort that array by decoration.spec ? (or by the recorded
codeUnitFrom values) ascending before returning Decoration.set(sortedArray).
Ensure you reference buildDecorations, spanInBytesToSpanInCodeUnits,
yellowHighlight.range and Decoration.set when making the change.
In `@src/playground/tabs/GritQLSearchTab.tsx`:
- Around line 78-103: Replace the unstable index key on the mapped list items
with a stable key derived from the match tuple so React can track items
reliably; in the map over gritQueryResults.matches (inside GritQLSearchTab
render where spanInBytesToSpanInCodeUnits and handleMatchClick are used),
compute a key from the match tuple (e.g. combine match[0] and match[1] into a
string like `${match[0]}-${match[1]}` or use match.join("-")) and use that
instead of index, ensuring uniqueness for each [number, number] byte-range
match.
- Around line 87-99: The clickable list item in the GritQLSearchTab component
lacks accessibility attributes; update the <li> rendered inside the match list
(the element using onClick and onKeyDown and calling handleMatchClick(match)) to
include role="button" and tabIndex={0} so it is announced and focusable by
assistive tech/keyboard users; keep the existing onKeyDown behavior and ensure
the same element still calls handleMatchClick(match) on Enter/Space.
In `@src/playground/utils.ts`:
- Around line 428-454: byteOffsetToCodeUnitOffset miscounts UTF-8 4-byte
characters because it uses text.codePointAt(codeUnitIndex) but doesn't advance
past the low surrogate; update byteOffsetToCodeUnitOffset to mirror the
surrogate handling used in spanInBytesToSpanInCodeUnits: when you compute
byteLength and determine it equals 4, increment the codeUnitIndex an extra time
(skip the low surrogate) before continuing the loop (or reuse/call
spanInBytesToSpanInCodeUnits logic) so byteIndex and returned codeUnitIndex
correctly reflect surrogate pairs (function: byteOffsetToCodeUnitOffset;
related: spanInBytesToSpanInCodeUnits).
In `@src/playground/workers/biomeWorker.ts`:
- Around line 428-461: The current block swallows errors from
parsePattern/searchPattern/dropPattern so BiomeOutput.gritQuery.error is never
set; modify the logic around gritQueryMatches/gritQuery so that any caught
exception from workspace.parsePattern, workspace.searchPattern, or
workspace.dropPattern is captured and assigned to the output (e.g., set
BiomeOutput.gritQuery.error to the error message or object) and only continue to
subsequent steps when safe (use patternId only if parse succeeds); update error
handling in the try/catch around parsePattern, searchPattern, and dropPattern to
populate BiomeOutput.gritQuery.error rather than just logging to console.
🧹 Nitpick comments (4)
src/playground/Playground.tsx (1)
184-184: Consider usinguseRefinstead ofcreateRef.
createRefcreates a new ref object on every render, whereasuseRefpersists across renders. This is a pre-existing pattern in the file, but worth noting if you're touching this area.src/playground/tabs/GritQLSearchTab.tsx (2)
80-84: Repeated byte-to-code-unit conversion on every render.
spanInBytesToSpanInCodeUnitsis called here during render and again inhandleMatchClick. For large match lists, consider memoising the converted spans to avoid redundant computation.
98-98: Snippet may render trailing "..." even when full text is shown.When the match is shorter than 50 characters, the ellipsis is misleading. Consider conditionally appending it only when truncation occurs.
Proposed fix
- <span className="gritql-match-snippet">{snippet}...</span> + <span className="gritql-match-snippet"> + {snippet}{to - from > 50 ? "..." : ""} + </span>src/playground/PlaygroundLoader.tsx (1)
257-264: Function call in dependency array may cause unnecessary re-renders.
getCurrentCode(state)andgetFileState(state, state.currentFile)?.gritQueryare function calls that compute new values on every render, potentially triggering the effect more than intended. Consider extracting these to variables or usinguseMemofor the values.
| function buildDecorations( | ||
| matches: [number, number][], | ||
| doc: string, | ||
| ): DecorationSet { | ||
| const decorations: any[] = []; | ||
|
|
||
| for (const [from, to] of matches) { | ||
| const [codeUnitFrom, codeUnitTo] = spanInBytesToSpanInCodeUnits( | ||
| [from, to], | ||
| doc, | ||
| ); | ||
| decorations.push(yellowHighlight.range(codeUnitFrom, codeUnitTo)); | ||
| } | ||
|
|
||
| return Decoration.set(decorations); | ||
| } |
There was a problem hiding this comment.
Decorations must be sorted by position for Decoration.set.
CodeMirror's Decoration.set requires decorations to be sorted by from position. If matches arrive out of order, this will throw a runtime error.
Proposed fix
function buildDecorations(
matches: [number, number][],
doc: string,
): DecorationSet {
- const decorations: any[] = [];
+ const decorations: { from: number; to: number }[] = [];
for (const [from, to] of matches) {
const [codeUnitFrom, codeUnitTo] = spanInBytesToSpanInCodeUnits(
[from, to],
doc,
);
- decorations.push(yellowHighlight.range(codeUnitFrom, codeUnitTo));
+ decorations.push({ from: codeUnitFrom, to: codeUnitTo });
}
- return Decoration.set(decorations);
+ decorations.sort((a, b) => a.from - b.from);
+ return Decoration.set(decorations.map((d) => yellowHighlight.range(d.from, d.to)));
}This also addresses the pipeline warning about any[] by using a typed intermediate array.
🧰 Tools
🪛 GitHub Actions: Integrate
[warning] 25-25: Unexpected any type. Consider a more specific type instead of any.
🤖 Prompt for AI Agents
In `@src/playground/CodeMirror.tsx` around lines 21 - 36, buildDecorations
currently pushes Decoration objects in the incoming match order which can be
unsorted and causes Decoration.set to throw; update buildDecorations to collect
typed Decoration objects (e.g., Decoration[] instead of any[]) and sort them by
their numeric start position before calling Decoration.set: for each match call
spanInBytesToSpanInCodeUnits([from,to], doc) to get codeUnitFrom/codeUnitTo,
create the decoration via yellowHighlight.range(codeUnitFrom, codeUnitTo), push
to an array typed as Decoration[], then sort that array by decoration.spec ? (or
by the recorded codeUnitFrom values) ascending before returning
Decoration.set(sortedArray). Ensure you reference buildDecorations,
spanInBytesToSpanInCodeUnits, yellowHighlight.range and Decoration.set when
making the change.
| if (gritQueryMatches && gritQueryMatches.length > 0) { | ||
| return [gritQueryMatchesField, ...baseExtensions]; | ||
| } |
There was a problem hiding this comment.
Decoration field not included when gritQueryMatches is empty.
The condition gritQueryMatches.length > 0 means the field is omitted when no matches exist. If matches later become non-empty, the field won't be present to receive the annotation, causing the highlights to not appear until a full extension rebuild. Consider always including the field when gritQueryMatches is defined (even if empty).
Proposed fix
- if (gritQueryMatches && gritQueryMatches.length > 0) {
+ if (gritQueryMatches !== undefined) {
return [gritQueryMatchesField, ...baseExtensions];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (gritQueryMatches && gritQueryMatches.length > 0) { | |
| return [gritQueryMatchesField, ...baseExtensions]; | |
| } | |
| if (gritQueryMatches !== undefined) { | |
| return [gritQueryMatchesField, ...baseExtensions]; | |
| } |
🤖 Prompt for AI Agents
In `@src/playground/CodeMirror.tsx` around lines 133 - 135, The code currently
only returns gritQueryMatchesField when gritQueryMatches.length > 0, so when
gritQueryMatches is defined but empty the DecorationField
(gritQueryMatchesField) is omitted and cannot later receive annotations; change
the condition to include gritQueryMatchesField whenever gritQueryMatches is
defined (i.e., gritQueryMatches !== undefined or truthy but allow empty array)
so the returned extensions are [gritQueryMatchesField, ...baseExtensions]
whenever gritQueryMatches exists, ensuring the field is present to accept future
annotations without needing a full rebuild; update the branch that builds the
extensions around gritQueryMatches, referencing gritQueryMatches,
gritQueryMatchesField, and baseExtensions.
| {gritQueryResults.matches.length > 0 && ( | ||
| <ul className="gritql-match-list"> | ||
| {gritQueryResults.matches.map((match, index) => { | ||
| const [from, to] = spanInBytesToSpanInCodeUnits(match, code); | ||
| const lines = code.substring(0, from).split("\n"); | ||
| const lineNumber = lines.length; | ||
| const snippet = code.substring(from, Math.min(to, from + 50)); | ||
|
|
||
| return ( | ||
| <li | ||
| key={index} | ||
| className="gritql-match-item" | ||
| onClick={() => handleMatchClick(match)} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| handleMatchClick(match); | ||
| } | ||
| }} | ||
| > | ||
| <span className="gritql-match-line">Line {lineNumber}</span> | ||
| <span className="gritql-match-snippet">{snippet}...</span> | ||
| </li> | ||
| ); | ||
| })} | ||
| </ul> | ||
| )} |
There was a problem hiding this comment.
Use a stable key instead of array index.
The pipeline flagged this: using index as the key prop can cause issues if the list is reordered or filtered. Since matches are [number, number] tuples representing unique byte ranges, you could derive a stable key from the match itself.
Proposed fix
- <li
- key={index}
+ <li
+ key={`${match[0]}-${match[1]}`}
className="gritql-match-item"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {gritQueryResults.matches.length > 0 && ( | |
| <ul className="gritql-match-list"> | |
| {gritQueryResults.matches.map((match, index) => { | |
| const [from, to] = spanInBytesToSpanInCodeUnits(match, code); | |
| const lines = code.substring(0, from).split("\n"); | |
| const lineNumber = lines.length; | |
| const snippet = code.substring(from, Math.min(to, from + 50)); | |
| return ( | |
| <li | |
| key={index} | |
| className="gritql-match-item" | |
| onClick={() => handleMatchClick(match)} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| handleMatchClick(match); | |
| } | |
| }} | |
| > | |
| <span className="gritql-match-line">Line {lineNumber}</span> | |
| <span className="gritql-match-snippet">{snippet}...</span> | |
| </li> | |
| ); | |
| })} | |
| </ul> | |
| )} | |
| {gritQueryResults.matches.length > 0 && ( | |
| <ul className="gritql-match-list"> | |
| {gritQueryResults.matches.map((match, index) => { | |
| const [from, to] = spanInBytesToSpanInCodeUnits(match, code); | |
| const lines = code.substring(0, from).split("\n"); | |
| const lineNumber = lines.length; | |
| const snippet = code.substring(from, Math.min(to, from + 50)); | |
| return ( | |
| <li | |
| key={`${match[0]}-${match[1]}`} | |
| className="gritql-match-item" | |
| onClick={() => handleMatchClick(match)} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| handleMatchClick(match); | |
| } | |
| }} | |
| > | |
| <span className="gritql-match-line">Line {lineNumber}</span> | |
| <span className="gritql-match-snippet">{snippet}...</span> | |
| </li> | |
| ); | |
| })} | |
| </ul> | |
| )} |
🧰 Tools
🪛 GitHub Actions: Integrate
[warning] 88-89: Avoid using the index of an array as key property in a list (noArrayIndexKey).
🤖 Prompt for AI Agents
In `@src/playground/tabs/GritQLSearchTab.tsx` around lines 78 - 103, Replace the
unstable index key on the mapped list items with a stable key derived from the
match tuple so React can track items reliably; in the map over
gritQueryResults.matches (inside GritQLSearchTab render where
spanInBytesToSpanInCodeUnits and handleMatchClick are used), compute a key from
the match tuple (e.g. combine match[0] and match[1] into a string like
`${match[0]}-${match[1]}` or use match.join("-")) and use that instead of index,
ensuring uniqueness for each [number, number] byte-range match.
| <li | ||
| key={index} | ||
| className="gritql-match-item" | ||
| onClick={() => handleMatchClick(match)} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| handleMatchClick(match); | ||
| } | ||
| }} | ||
| > | ||
| <span className="gritql-match-line">Line {lineNumber}</span> | ||
| <span className="gritql-match-snippet">{snippet}...</span> | ||
| </li> |
There was a problem hiding this comment.
Missing accessibility attributes on interactive list item.
The <li> is clickable and keyboard-navigable but lacks role="button" and tabIndex={0} for screen reader and keyboard users.
Proposed fix
<li
key={`${match[0]}-${match[1]}`}
className="gritql-match-item"
+ role="button"
+ tabIndex={0}
onClick={() => handleMatchClick(match)}
onKeyDown={(e) => {🧰 Tools
🪛 GitHub Actions: Integrate
[warning] 88-89: Avoid using the index of an array as key property in a list (noArrayIndexKey).
🤖 Prompt for AI Agents
In `@src/playground/tabs/GritQLSearchTab.tsx` around lines 87 - 99, The clickable
list item in the GritQLSearchTab component lacks accessibility attributes;
update the <li> rendered inside the match list (the element using onClick and
onKeyDown and calling handleMatchClick(match)) to include role="button" and
tabIndex={0} so it is announced and focusable by assistive tech/keyboard users;
keep the existing onKeyDown behavior and ensure the same element still calls
handleMatchClick(match) on Enter/Space.
| export function byteOffsetToCodeUnitOffset( | ||
| byteOffset: number, | ||
| text: string, | ||
| ): number { | ||
| let byteIndex = 0; | ||
| let codeUnitIndex = 0; | ||
|
|
||
| for (codeUnitIndex = 0; codeUnitIndex < text.length; codeUnitIndex++) { | ||
| const codePoint = text.codePointAt(codeUnitIndex) ?? 0; | ||
| const byteLength = | ||
| codePoint <= 0x7f | ||
| ? 1 | ||
| : codePoint <= 0x7ff | ||
| ? 2 | ||
| : codePoint <= 0xffff | ||
| ? 3 | ||
| : 4; | ||
|
|
||
| if (byteIndex + byteLength > byteOffset) { | ||
| break; | ||
| } | ||
|
|
||
| byteIndex += byteLength; | ||
| } | ||
|
|
||
| return codeUnitIndex; | ||
| } |
There was a problem hiding this comment.
Surrogate pair handling is incorrect for 4-byte UTF-8 characters.
When iterating with codePointAt(), encountering a high surrogate returns the full code point (4-byte UTF-8). However, the loop doesn't skip the subsequent low surrogate, causing incorrect byte counting for characters like emojis.
The existing spanInBytesToSpanInCodeUnits handles this by incrementing currCodeUnitIndex when byteLength === 4. Consider reusing that function or applying a similar fix here.
🐛 Proposed fix to handle surrogate pairs
export function byteOffsetToCodeUnitOffset(
byteOffset: number,
text: string,
): number {
let byteIndex = 0;
let codeUnitIndex = 0;
for (codeUnitIndex = 0; codeUnitIndex < text.length; codeUnitIndex++) {
const codePoint = text.codePointAt(codeUnitIndex) ?? 0;
const byteLength =
codePoint <= 0x7f
? 1
: codePoint <= 0x7ff
? 2
: codePoint <= 0xffff
? 3
: 4;
if (byteIndex + byteLength > byteOffset) {
break;
}
byteIndex += byteLength;
+
+ // Skip the low surrogate for 4-byte characters (surrogate pairs)
+ if (byteLength === 4) {
+ codeUnitIndex++;
+ }
}
return codeUnitIndex;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function byteOffsetToCodeUnitOffset( | |
| byteOffset: number, | |
| text: string, | |
| ): number { | |
| let byteIndex = 0; | |
| let codeUnitIndex = 0; | |
| for (codeUnitIndex = 0; codeUnitIndex < text.length; codeUnitIndex++) { | |
| const codePoint = text.codePointAt(codeUnitIndex) ?? 0; | |
| const byteLength = | |
| codePoint <= 0x7f | |
| ? 1 | |
| : codePoint <= 0x7ff | |
| ? 2 | |
| : codePoint <= 0xffff | |
| ? 3 | |
| : 4; | |
| if (byteIndex + byteLength > byteOffset) { | |
| break; | |
| } | |
| byteIndex += byteLength; | |
| } | |
| return codeUnitIndex; | |
| } | |
| export function byteOffsetToCodeUnitOffset( | |
| byteOffset: number, | |
| text: string, | |
| ): number { | |
| let byteIndex = 0; | |
| let codeUnitIndex = 0; | |
| for (codeUnitIndex = 0; codeUnitIndex < text.length; codeUnitIndex++) { | |
| const codePoint = text.codePointAt(codeUnitIndex) ?? 0; | |
| const byteLength = | |
| codePoint <= 0x7f | |
| ? 1 | |
| : codePoint <= 0x7ff | |
| ? 2 | |
| : codePoint <= 0xffff | |
| ? 3 | |
| : 4; | |
| if (byteIndex + byteLength > byteOffset) { | |
| break; | |
| } | |
| byteIndex += byteLength; | |
| // Skip the low surrogate for 4-byte characters (surrogate pairs) | |
| if (byteLength === 4) { | |
| codeUnitIndex++; | |
| } | |
| } | |
| return codeUnitIndex; | |
| } |
🤖 Prompt for AI Agents
In `@src/playground/utils.ts` around lines 428 - 454, byteOffsetToCodeUnitOffset
miscounts UTF-8 4-byte characters because it uses
text.codePointAt(codeUnitIndex) but doesn't advance past the low surrogate;
update byteOffsetToCodeUnitOffset to mirror the surrogate handling used in
spanInBytesToSpanInCodeUnits: when you compute byteLength and determine it
equals 4, increment the codeUnitIndex an extra time (skip the low surrogate)
before continuing the loop (or reuse/call spanInBytesToSpanInCodeUnits logic) so
byteIndex and returned codeUnitIndex correctly reflect surrogate pairs
(function: byteOffsetToCodeUnitOffset; related: spanInBytesToSpanInCodeUnits).
| let gritQueryMatches: [number, number][] = []; | ||
| if (gritQuery) { | ||
| let patternId: string | null = null; | ||
| try { | ||
| const parseResult = workspace.parsePattern({ | ||
| pattern: String(gritQuery), | ||
| defaultLanguage: defaultLanguage || "JavaScript", | ||
| }); | ||
| patternId = String(parseResult.patternId); | ||
| } catch (e) { | ||
| console.error("Failed to parse GritQL query:", e); | ||
| } | ||
|
|
||
| if (patternId) { | ||
| try { | ||
| const searchResults = workspace.searchPattern({ | ||
| path: `/${filename}`, | ||
| pattern: patternId, | ||
| projectKey, | ||
| }); | ||
| gritQueryMatches = searchResults.matches || []; | ||
| } catch (e) { | ||
| console.error("Failed to search with GritQL query:", e); | ||
| } | ||
|
|
||
| try { | ||
| workspace.dropPattern({ | ||
| pattern: patternId, | ||
| }); | ||
| } catch (e) { | ||
| console.warn("Failed to drop pattern:", e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Errors aren't propagated to the UI.
The BiomeOutput.gritQuery.error field exists in the types but is never populated here. Users won't see why their query failed.
🔧 Proposed fix to propagate errors
- let gritQueryMatches: [number, number][] = [];
+ let gritQueryMatches: [number, number][] = [];
+ let gritQueryError: string | undefined;
if (gritQuery) {
let patternId: string | null = null;
try {
const parseResult = workspace.parsePattern({
pattern: String(gritQuery),
defaultLanguage: defaultLanguage || "JavaScript",
});
patternId = String(parseResult.patternId);
} catch (e) {
console.error("Failed to parse GritQL query:", e);
+ gritQueryError = e instanceof Error ? e.message : String(e);
}
if (patternId) {
try {
const searchResults = workspace.searchPattern({
path: `/${filename}`,
pattern: patternId,
projectKey,
});
gritQueryMatches = searchResults.matches || [];
} catch (e) {
console.error("Failed to search with GritQL query:", e);
+ gritQueryError = e instanceof Error ? e.message : String(e);
}Then update the output:
gritQuery: {
matches: gritQueryMatches,
+ error: gritQueryError,
},🤖 Prompt for AI Agents
In `@src/playground/workers/biomeWorker.ts` around lines 428 - 461, The current
block swallows errors from parsePattern/searchPattern/dropPattern so
BiomeOutput.gritQuery.error is never set; modify the logic around
gritQueryMatches/gritQuery so that any caught exception from
workspace.parsePattern, workspace.searchPattern, or workspace.dropPattern is
captured and assigned to the output (e.g., set BiomeOutput.gritQuery.error to
the error message or object) and only continue to subsequent steps when safe
(use patternId only if parse succeeds); update error handling in the try/catch
around parsePattern, searchPattern, and dropPattern to populate
BiomeOutput.gritQuery.error rather than just logging to console.
dyc3
force-pushed
the
dyc3/grit-in-playground
branch
from
a15b47e to
25015d8
Compare
1 participant
Summary
This PR makes it so that users can test grit queries in the playground.
Also, this will let users file bugs for grit query issues easier.
A lot of this was AI generated by glm-4.7