Merged
Conversation
|
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Jan 19, 2026
Contributor
WalkthroughAdds a new package Possibly related PRs
Suggested labels
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
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/`@biomejs/prettier-compare/src/components/App.tsx:
- Around line 112-119: The "rebuilt" async handler registered on watcher (the
watcher.on("rebuilt", async () => { ... })) lacks error handling and can leave
state inconsistent if reloadBiome() or runComparison() throws; wrap the handler
body in a try/catch/finally: in try, keep the existing
setStatusMessage("Reloading Biome..."), await reloadBiome() and await
runComparison(); in catch, log the error and update status with an error
message; in finally, always call setIsRebuilding(false) and
setRebuildingFile(null) and clear status as appropriate so state is reset even
on failure. Ensure you reference the existing functions reloadBiome and
runComparison and the state setters setIsRebuilding/setRebuildingFile when
applying the fix.
In `@packages/`@biomejs/prettier-compare/src/languages.ts:
- Around line 98-102: The languages configuration lists a "graphql" parser but
there's no plugin wiring; update either the languages.ts entry or the plugin
resolution: either remove the "graphql" key from the languages map (if GraphQL
support is not intended) or add `@prettier/plugin-graphql` as a dependency and
modify getPluginsForParser in prettier.ts to return/include the GraphQL plugin
when parser === "graphql" (similar to how Svelte is handled) so the parser has
an actual plugin implementation.
In `@packages/`@biomejs/prettier-compare/src/watch.ts:
- Around line 58-113: The watcher currently drops changes that occur while
isRebuilding is true; update the watcher.on("all") handler to queue a single
pending rebuild instead of skipping: introduce a pendingRebuild boolean
alongside debounceTimer and isRebuilding, set pendingRebuild = true when a new
.rs event arrives while isRebuilding is true (or during the debounce window),
and after the try/finally that resets isRebuilding in the rebuild block, check
pendingRebuild — if true, clear pendingRebuild and run the rebuild flow again
(respecting DEBOUNCE_MS or directly invoking rebuildWasm and emitting the same
events) so one extra rebuild is guaranteed to run; refer to debounceTimer,
isRebuilding, pendingRebuild, watcher.on("all"), rebuildWasm, emitter.emit, and
DEBOUNCE_MS when making the change.
🧹 Nitpick comments (3)
packages/@biomejs/prettier-compare/src/prettier.ts (1)
52-63: Surface IR unavailability rather than returning an empty string.
When__debugis absent,irstays"", which looks like “no diff” rather than “not supported”. Consider setting the same placeholder used in the catch.♻️ Proposed tweak
- try { + try { const debugApi = (prettier as unknown as { __debug: PrettierDebugApi }) .__debug; - if (debugApi?.printToDoc && debugApi?.formatDoc) { + if (debugApi?.printToDoc && debugApi?.formatDoc) { const doc = await debugApi.printToDoc(code, options); ir = await debugApi.formatDoc(doc, {}); + } else { + ir = "(IR extraction not available)"; } } catch { // IR extraction is best-effort; don't fail if it doesn't work ir = "(IR extraction not available)"; }packages/@biomejs/prettier-compare/src/index.tsx (1)
33-39: Consider trimming trailing newline from stdin.When piping input, a trailing newline is often appended. You might want to trim it to match the behaviour of direct snippet input:
♻️ Optional fix
async function readStdin(): Promise<string> { const chunks: Buffer[] = []; for await (const chunk of process.stdin) { chunks.push(chunk as Buffer); } - return Buffer.concat(chunks).toString("utf-8"); + return Buffer.concat(chunks).toString("utf-8").trimEnd(); }packages/@biomejs/prettier-compare/src/components/App.tsx (1)
76-98: Effect may re-run unnecessarily whencodeorlanguagechanges.Since
runComparisondepends oncodeandlanguage, any change to those props will re-run this entire effect, including the rebuild logic (ifrebuildis true). This could trigger redundant WASM rebuilds on prop changes.Consider separating the initial rebuild into its own effect with only
[rebuild, rootDir]as dependencies, or using a ref to track whether the initial rebuild has already completed.♻️ Suggested approach
+ const hasInitialized = useRef(false); + // Initial setup: optionally rebuild WASM, then run comparison useEffect(() => { async function initialize() { - if (rebuild) { + if (rebuild && !hasInitialized.current) { + hasInitialized.current = true; setStatusMessage("Rebuilding WASM (--rebuild flag)..."); // ... rest of rebuild logic } await runComparison(); } initialize(); }, [rebuild, rootDir, runComparison]);
Comment on lines
112
to
119
| watcher.on("rebuilt", async () => { | ||
| setStatusMessage("Reloading Biome..."); | ||
| await reloadBiome(); | ||
| setStatusMessage(null); | ||
| await runComparison(); | ||
| setIsRebuilding(false); | ||
| setRebuildingFile(null); | ||
| }); |
Contributor
There was a problem hiding this comment.
Missing error handling in async event handler.
If reloadBiome() or runComparison() throws, the promise rejection is unhandled, and the UI may be left in an inconsistent state (e.g., isRebuilding remains true).
🐛 Proposed fix
watcher.on("rebuilt", async () => {
+ try {
setStatusMessage("Reloading Biome...");
await reloadBiome();
setStatusMessage(null);
await runComparison();
- setIsRebuilding(false);
- setRebuildingFile(null);
+ } catch (err) {
+ setError(`Reload failed: ${err instanceof Error ? err.message : String(err)}`);
+ } finally {
+ setIsRebuilding(false);
+ setRebuildingFile(null);
+ }
});🤖 Prompt for AI Agents
In `@packages/`@biomejs/prettier-compare/src/components/App.tsx around lines 112 -
119, The "rebuilt" async handler registered on watcher (the
watcher.on("rebuilt", async () => { ... })) lacks error handling and can leave
state inconsistent if reloadBiome() or runComparison() throws; wrap the handler
body in a try/catch/finally: in try, keep the existing
setStatusMessage("Reloading Biome..."), await reloadBiome() and await
runComparison(); in catch, log the error and update status with an error
message; in finally, always call setIsRebuilding(false) and
setRebuildingFile(null) and clear status as appropriate so state is reset even
on failure. Ensure you reference the existing functions reloadBiome and
runComparison and the state setters setIsRebuilding/setRebuildingFile when
applying the fix.
Comment on lines
+98
to
+102
| svelte: { | ||
| biomeFilePath: "file.svelte", | ||
| prettierParser: "svelte", | ||
| displayName: "Svelte", | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if getPluginsForParser handles svelte and graphql parsers
echo "=== getPluginsForParser function ==="
rg -n -A 20 "getPluginsForParser" packages/@biomejs/prettier-compare/src/prettier.ts
echo -e "\n=== Package.json dependencies ==="
cat packages/@biomejs/prettier-compare/package.json | jq '.dependencies, .devDependencies' 2>/dev/null || cat packages/@biomejs/prettier-compare/package.jsonRepository: biomejs/biome
Length of output: 1772
🏁 Script executed:
#!/bin/bash
# Check if graphql is defined in languages.ts
rg -n "graphql" packages/@biomejs/prettier-compare/src/languages.tsRepository: biomejs/biome
Length of output: 207
GraphQL parser is defined but missing plugin implementation.
The graphql parser is configured in languages.ts but @prettier/plugin-graphql is neither in package.json nor handled by getPluginsForParser in prettier.ts (which currently only handles Svelte). Either add the GraphQL plugin dependency and handler, or remove the GraphQL configuration if it's not meant to be supported yet.
(Svelte is already correctly handled.)
🤖 Prompt for AI Agents
In `@packages/`@biomejs/prettier-compare/src/languages.ts around lines 98 - 102,
The languages configuration lists a "graphql" parser but there's no plugin
wiring; update either the languages.ts entry or the plugin resolution: either
remove the "graphql" key from the languages map (if GraphQL support is not
intended) or add `@prettier/plugin-graphql` as a dependency and modify
getPluginsForParser in prettier.ts to return/include the GraphQL plugin when
parser === "graphql" (similar to how Svelte is handled) so the parser has an
actual plugin implementation.
Comment on lines
58
to
113
| let debounceTimer: NodeJS.Timeout | null = null; | ||
| let isRebuilding = false; | ||
|
|
||
| // Convert relative directories to absolute paths | ||
| const absoluteDirs = WATCH_DIRS.map((dir) => resolve(rootDir, dir)); | ||
|
|
||
| debug("Creating watcher for directories:", absoluteDirs); | ||
|
|
||
| const watcher = chokidar.watch(absoluteDirs, { | ||
| ignoreInitial: true, | ||
| // Ignore build artifacts and non-source files | ||
| ignored: ["**/target/**", "**/node_modules/**"], | ||
| }); | ||
|
|
||
| watcher.on("ready", () => { | ||
| debug("Watcher ready"); | ||
| const watched = watcher.getWatched(); | ||
| debug("Watching", Object.keys(watched).length, "directories"); | ||
| }); | ||
|
|
||
| // Listen to all events (add, change, unlink) and filter for .rs files | ||
| watcher.on("all", (event, path) => { | ||
| // Only react to Rust file changes | ||
| if (!path.endsWith(".rs")) { | ||
| return; | ||
| } | ||
|
|
||
| debug(`File ${event}:`, path); | ||
|
|
||
| // Clear any pending debounce timer | ||
| if (debounceTimer) { | ||
| clearTimeout(debounceTimer); | ||
| } | ||
|
|
||
| // Debounce rapid file changes | ||
| debounceTimer = setTimeout(async () => { | ||
| if (isRebuilding) { | ||
| debug("Already rebuilding, skipping"); | ||
| return; | ||
| } | ||
|
|
||
| isRebuilding = true; | ||
| emitter.emit("rebuilding", path); | ||
|
|
||
| try { | ||
| await rebuildWasm(rootDir); | ||
| emitter.emit("rebuilt"); | ||
| } catch (err) { | ||
| emitter.emit( | ||
| "error", | ||
| err instanceof Error ? err : new Error(String(err)), | ||
| ); | ||
| } finally { | ||
| isRebuilding = false; | ||
| } | ||
| }, DEBOUNCE_MS); |
Contributor
There was a problem hiding this comment.
Rebuilds can be skipped while a build is running.
If changes happen during a rebuild, they’re dropped (Line 94–97), so the latest edit may never be built. Consider queueing one extra rebuild.
🛠️ One way to queue a rebuild
- let debounceTimer: NodeJS.Timeout | null = null;
- let isRebuilding = false;
+ let debounceTimer: NodeJS.Timeout | null = null;
+ let isRebuilding = false;
+ let pendingRebuild = false;
+ let lastChangedPath: string | null = null;
+
+ const runRebuild = async (changedFile: string) => {
+ isRebuilding = true;
+ emitter.emit("rebuilding", changedFile);
+ try {
+ await rebuildWasm(rootDir);
+ emitter.emit("rebuilt");
+ } catch (err) {
+ emitter.emit("error", err instanceof Error ? err : new Error(String(err)));
+ } finally {
+ isRebuilding = false;
+ if (pendingRebuild) {
+ pendingRebuild = false;
+ const queued = lastChangedPath ?? changedFile;
+ lastChangedPath = null;
+ await runRebuild(queued);
+ }
+ }
+ };
@@
- debounceTimer = setTimeout(async () => {
- if (isRebuilding) {
- debug("Already rebuilding, skipping");
- return;
- }
-
- isRebuilding = true;
- emitter.emit("rebuilding", path);
-
- try {
- await rebuildWasm(rootDir);
- emitter.emit("rebuilt");
- } catch (err) {
- emitter.emit(
- "error",
- err instanceof Error ? err : new Error(String(err)),
- );
- } finally {
- isRebuilding = false;
- }
- }, DEBOUNCE_MS);
+ debounceTimer = setTimeout(async () => {
+ if (isRebuilding) {
+ pendingRebuild = true;
+ lastChangedPath = path;
+ debug("Already rebuilding, queueing");
+ return;
+ }
+ await runRebuild(path);
+ }, DEBOUNCE_MS);🤖 Prompt for AI Agents
In `@packages/`@biomejs/prettier-compare/src/watch.ts around lines 58 - 113, The
watcher currently drops changes that occur while isRebuilding is true; update
the watcher.on("all") handler to queue a single pending rebuild instead of
skipping: introduce a pendingRebuild boolean alongside debounceTimer and
isRebuilding, set pendingRebuild = true when a new .rs event arrives while
isRebuilding is true (or during the debounce window), and after the try/finally
that resets isRebuilding in the rebuild block, check pendingRebuild — if true,
clear pendingRebuild and run the rebuild flow again (respecting DEBOUNCE_MS or
directly invoking rebuildWasm and emitting the same events) so one extra rebuild
is guaranteed to run; refer to debounceTimer, isRebuilding, pendingRebuild,
watcher.on("all"), rebuildWasm, emitter.emit, and DEBOUNCE_MS when making the
change.
ematipico
approved these changes
Jan 19, 2026
Member
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
We could lean towards Node.js a bit more to reduce the number of deps.
dyc3
force-pushed
the
dyc3/prettier-compare-tool
branch
from
13103d8 to
cef058c
Compare
github-actions
bot
added
A-Parser
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/prettier-compare/README.md`:
- Line 10: Update the architecture diagram entry that currently reads
"packages/@biomejs/prettier-compare/" to the actual package path
"packages/prettier-compare/" (as indicated by package.json); search the
README.md for the incorrect string "packages/@biomejs/prettier-compare/" and
replace it with "packages/prettier-compare/", and ensure any other occurrences
in the diagram or surrounding documentation are updated to match.
In `@packages/prettier-compare/src/index.tsx`:
- Around line 87-120: The CLI currently allows both --ir-only and --output-only
together which yields no useful output; in parseCliArgs detect when
values["ir-only"] and values["output-only"] are both true and reject this
combination early (e.g., print an error message via console.error or a logger,
call printHelp() if helpful, and exit with non‑zero status) so the program fails
fast; update parseCliArgs to validate those flags before returning and ensure
the returned NormalizedCliOptions is only produced when the combination is
valid.
In `@packages/prettier-compare/src/watch.ts`:
- Around line 61-99: The debounce logic can drop changes that occur while
isRebuilding is true; update scheduleRebuild to introduce a pendingRebuild
boolean (e.g., let pendingRebuild = false) and when a change arrives while
isRebuilding is true set pendingRebuild = true (and update lastChangedFile), and
in the finally block after isRebuilding = false check pendingRebuild: if true,
clear pendingRebuild and call scheduleRebuild(lastChangedFile!) (or re-trigger
the debounce) so the change recorded during rebuild will schedule a follow-up
rebuild; ensure emitter events and debounceTimer behavior remain unchanged and
that rebuildWasm is still awaited and errors forwarded via emitter.emit("error",
toError(err)).
♻️ Duplicate comments (2)
packages/prettier-compare/src/languages.ts (1)
68-77: GraphQL configuration lacks corresponding Prettier plugin.The
graphqlparser is configured here, but@prettier/plugin-graphqlisn't inpackage.jsonand isn't handled inprettier.ts. Formatting GraphQL will fail at runtime.Either add the plugin dependency and wire it up, or remove GraphQL support for now.
packages/prettier-compare/src/components/App.tsx (1)
112-119: Guard the rebuilt handler so state always resets.
IfrunComparison()throws, the rejection is unhandled andisRebuilding/rebuildingFilenever reset. Wrap in try/catch/finally.🐛 Proposed fix
watcher.on("rebuilt", async () => { - setStatusMessage("Reloading Biome..."); - // reloading biome is handled by bun hot reloading - setStatusMessage(null); - await runComparison(); - setIsRebuilding(false); - setRebuildingFile(null); + try { + setStatusMessage("Reloading Biome..."); + // reloading biome is handled by bun hot reloading + setStatusMessage(null); + await runComparison(); + } catch (err) { + setError( + `Reload failed: ${err instanceof Error ? err.message : String(err)}`, + ); + } finally { + setIsRebuilding(false); + setRebuildingFile(null); + } });Please verify by forcing a formatter error and confirming the spinner clears and state resets.
Comment on lines
+87
to
+120
| function parseCliArgs(): { snippet?: string; options: NormalizedCliOptions } { | ||
| const { values, positionals } = parseArgs({ | ||
| args: process.argv.slice(2), | ||
| allowPositionals: true, | ||
| options: { | ||
| file: { type: "string", short: "f" }, | ||
| language: { type: "string", short: "l" }, | ||
| watch: { type: "boolean", short: "w", default: false }, | ||
| rebuild: { type: "boolean", short: "r", default: false }, | ||
| "ir-only": { type: "boolean", default: false }, | ||
| "output-only": { type: "boolean", default: false }, | ||
| help: { type: "boolean", short: "h", default: false }, | ||
| }, | ||
| }) as { | ||
| values: RawCliOptionValues; | ||
| positionals: string[]; | ||
| }; | ||
|
|
||
| if (values.help) { | ||
| printHelp(); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| return { | ||
| snippet: positionals[0], | ||
| options: { | ||
| file: values.file, | ||
| language: values.language, | ||
| watch: values.watch ?? false, | ||
| rebuild: values.rebuild ?? false, | ||
| irOnly: values["ir-only"] ?? false, | ||
| outputOnly: values["output-only"] ?? false, | ||
| }, | ||
| }; |
Contributor
There was a problem hiding this comment.
Prevent --ir-only and --output-only being combined.
As-is, both flags together produce no main output, which is confusing. Consider rejecting that combination early.
💡 Suggested guard
function parseCliArgs(): { snippet?: string; options: NormalizedCliOptions } {
const { values, positionals } = parseArgs({
@@
if (values.help) {
printHelp();
process.exit(0);
}
+ const irOnly = values["ir-only"] ?? false;
+ const outputOnly = values["output-only"] ?? false;
+ if (irOnly && outputOnly) {
+ console.error("Error: --ir-only and --output-only cannot be used together.");
+ process.exit(1);
+ }
return {
snippet: positionals[0],
options: {
file: values.file,
language: values.language,
watch: values.watch ?? false,
rebuild: values.rebuild ?? false,
- irOnly: values["ir-only"] ?? false,
- outputOnly: values["output-only"] ?? false,
+ irOnly,
+ outputOnly,
},
};
}🤖 Prompt for AI Agents
In `@packages/prettier-compare/src/index.tsx` around lines 87 - 120, The CLI
currently allows both --ir-only and --output-only together which yields no
useful output; in parseCliArgs detect when values["ir-only"] and
values["output-only"] are both true and reject this combination early (e.g.,
print an error message via console.error or a logger, call printHelp() if
helpful, and exit with non‑zero status) so the program fails fast; update
parseCliArgs to validate those flags before returning and ensure the returned
NormalizedCliOptions is only produced when the combination is valid.
Comment on lines
+61
to
+99
| const emitter = new EventEmitter(); | ||
| const watchers = new Map<string, FSWatcher>(); | ||
| let debounceTimer: NodeJS.Timeout | null = null; | ||
| let isRebuilding = false; | ||
| let lastChangedFile: string | null = null; | ||
|
|
||
| const absoluteDirs = WATCH_DIRS.map((dir) => resolve(rootDir, dir)); | ||
|
|
||
| const scheduleRebuild = (changedFile: string) => { | ||
| lastChangedFile = changedFile; | ||
|
|
||
| if (debounceTimer) { | ||
| clearTimeout(debounceTimer); | ||
| } | ||
|
|
||
| debounceTimer = setTimeout(async () => { | ||
| debounceTimer = null; | ||
|
|
||
| if (isRebuilding) { | ||
| return; | ||
| } | ||
|
|
||
| if (!lastChangedFile) { | ||
| return; | ||
| } | ||
|
|
||
| isRebuilding = true; | ||
| emitter.emit("rebuilding", lastChangedFile); | ||
|
|
||
| try { | ||
| await rebuildWasm(rootDir); | ||
| emitter.emit("rebuilt"); | ||
| } catch (err) { | ||
| emitter.emit("error", toError(err)); | ||
| } finally { | ||
| isRebuilding = false; | ||
| lastChangedFile = null; | ||
| } | ||
| }, DEBOUNCE_MS); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "watch.ts" -path "*/packages/prettier-compare/*"Repository: biomejs/biome
Length of output: 97
🏁 Script executed:
cat -n packages/prettier-compare/src/watch.tsRepository: biomejs/biome
Length of output: 7683
Rebuild can drop file changes that occur during an in-flight rebuild.
When a file changes while isRebuilding is true, the debounce timer fires, checks the flag, and returns early (line 79–80) without rescheduling. The change is recorded in lastChangedFile, but once the current rebuild completes, that change is discarded without triggering a follow-up rebuild.
Add a pendingRebuild flag to track whether a change occurred during the rebuild, then check it in the finally block and reschedule if needed.
Proposed fix
const watchers = new Map<string, FSWatcher>();
let debounceTimer: NodeJS.Timeout | null = null;
let isRebuilding = false;
let lastChangedFile: string | null = null;
+ let pendingRebuild = false;
const scheduleRebuild = (changedFile: string) => {
lastChangedFile = changedFile;
+
+ if (isRebuilding) {
+ pendingRebuild = true;
+ return;
+ }
if (debounceTimer) {
clearTimeout(debounceTimer);
}
debounceTimer = setTimeout(async () => {
debounceTimer = null;
if (isRebuilding) {
+ pendingRebuild = true;
return;
}
if (!lastChangedFile) {
return;
}
isRebuilding = true;
emitter.emit("rebuilding", lastChangedFile);
try {
await rebuildWasm(rootDir);
emitter.emit("rebuilt");
} catch (err) {
emitter.emit("error", toError(err));
} finally {
isRebuilding = false;
- lastChangedFile = null;
+ if (pendingRebuild && lastChangedFile) {
+ pendingRebuild = false;
+ scheduleRebuild(lastChangedFile);
+ } else {
+ lastChangedFile = null;
+ pendingRebuild = false;
+ }
}
}, DEBOUNCE_MS);
};📝 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.
Suggested change
| const emitter = new EventEmitter(); | |
| const watchers = new Map<string, FSWatcher>(); | |
| let debounceTimer: NodeJS.Timeout | null = null; | |
| let isRebuilding = false; | |
| let lastChangedFile: string | null = null; | |
| const absoluteDirs = WATCH_DIRS.map((dir) => resolve(rootDir, dir)); | |
| const scheduleRebuild = (changedFile: string) => { | |
| lastChangedFile = changedFile; | |
| if (debounceTimer) { | |
| clearTimeout(debounceTimer); | |
| } | |
| debounceTimer = setTimeout(async () => { | |
| debounceTimer = null; | |
| if (isRebuilding) { | |
| return; | |
| } | |
| if (!lastChangedFile) { | |
| return; | |
| } | |
| isRebuilding = true; | |
| emitter.emit("rebuilding", lastChangedFile); | |
| try { | |
| await rebuildWasm(rootDir); | |
| emitter.emit("rebuilt"); | |
| } catch (err) { | |
| emitter.emit("error", toError(err)); | |
| } finally { | |
| isRebuilding = false; | |
| lastChangedFile = null; | |
| } | |
| }, DEBOUNCE_MS); | |
| const emitter = new EventEmitter(); | |
| const watchers = new Map<string, FSWatcher>(); | |
| let debounceTimer: NodeJS.Timeout | null = null; | |
| let isRebuilding = false; | |
| let lastChangedFile: string | null = null; | |
| let pendingRebuild = false; | |
| const absoluteDirs = WATCH_DIRS.map((dir) => resolve(rootDir, dir)); | |
| const scheduleRebuild = (changedFile: string) => { | |
| lastChangedFile = changedFile; | |
| if (isRebuilding) { | |
| pendingRebuild = true; | |
| return; | |
| } | |
| if (debounceTimer) { | |
| clearTimeout(debounceTimer); | |
| } | |
| debounceTimer = setTimeout(async () => { | |
| debounceTimer = null; | |
| if (isRebuilding) { | |
| pendingRebuild = true; | |
| return; | |
| } | |
| if (!lastChangedFile) { | |
| return; | |
| } | |
| isRebuilding = true; | |
| emitter.emit("rebuilding", lastChangedFile); | |
| try { | |
| await rebuildWasm(rootDir); | |
| emitter.emit("rebuilt"); | |
| } catch (err) { | |
| emitter.emit("error", toError(err)); | |
| } finally { | |
| isRebuilding = false; | |
| if (pendingRebuild && lastChangedFile) { | |
| pendingRebuild = false; | |
| scheduleRebuild(lastChangedFile); | |
| } else { | |
| lastChangedFile = null; | |
| pendingRebuild = false; | |
| } | |
| } | |
| }, DEBOUNCE_MS); | |
| }; |
🤖 Prompt for AI Agents
In `@packages/prettier-compare/src/watch.ts` around lines 61 - 99, The debounce
logic can drop changes that occur while isRebuilding is true; update
scheduleRebuild to introduce a pendingRebuild boolean (e.g., let pendingRebuild
= false) and when a change arrives while isRebuilding is true set pendingRebuild
= true (and update lastChangedFile), and in the finally block after isRebuilding
= false check pendingRebuild: if true, clear pendingRebuild and call
scheduleRebuild(lastChangedFile!) (or re-trigger the debounce) so the change
recorded during rebuild will schedule a follow-up rebuild; ensure emitter events
and debounceTimer behavior remain unchanged and that rebuildWasm is still
awaited and errors forwarded via emitter.emit("error", toError(err)).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/prettier-compare/src/watch.ts`:
- Around line 233-259: In rebuildWasm, the child spawn uses stdio: "pipe" but
only consumes stderr, which can cause stdout back-pressure; change the spawn
options for the proc created in rebuildWasm (the spawn call that runs "just",
["build-wasm-node-dev"]) to use stdio: ["ignore", "inherit", "pipe"] so stdout
is inherited (streams to parent) while stderr remains piped and can still be
collected by the existing proc.stderr?.on("data") handler; leave the rest of the
error/close handling intact.
♻️ Duplicate comments (2)
packages/prettier-compare/src/watch.ts (1)
72-101: Don’t drop rebuild triggers during an active rebuild.
Line 82–84 returns early whileisRebuildingis true, and Line 99–100 clearslastChangedFile, so edits that land mid‑rebuild can be missed. Track a pending rebuild and reschedule once the current build finishes.🛠️ Suggested fix
const watchers = new Map<string, FSWatcher>(); let debounceTimer: NodeJS.Timeout | null = null; let isRebuilding = false; let lastChangedFile: string | null = null; +let pendingRebuild = false; const scheduleRebuild = (changedFile: string) => { lastChangedFile = changedFile; + + if (isRebuilding) { + pendingRebuild = true; + return; + } if (debounceTimer) { clearTimeout(debounceTimer); } debounceTimer = setTimeout(async () => { debounceTimer = null; if (isRebuilding) { + pendingRebuild = true; return; } @@ } finally { isRebuilding = false; - lastChangedFile = null; + if (pendingRebuild && lastChangedFile) { + pendingRebuild = false; + scheduleRebuild(lastChangedFile); + } else { + lastChangedFile = null; + pendingRebuild = false; + } } }, DEBOUNCE_MS); };packages/prettier-compare/src/index.tsx (1)
92-124: Guard against--ir-onlyand--output-onlytogether.
Line 115–123 allows both flags, which yields no main output. A quick fail‑fast makes the CLI less puzzling.💡 Suggested guard
if (values.help) { printHelp(); process.exit(0); } + + const irOnly = values["ir-only"] ?? false; + const outputOnly = values["output-only"] ?? false; + if (irOnly && outputOnly) { + console.error("Error: --ir-only and --output-only cannot be used together."); + process.exit(1); + } return { snippet: positionals[0], options: { file: values.file, language: values.language, watch: values.watch ?? false, rebuild: values.rebuild ?? false, - irOnly: values["ir-only"] ?? false, - outputOnly: values["output-only"] ?? false, + irOnly, + outputOnly, }, }; }
Comment on lines
+233
to
+259
| return new Promise((resolve, reject) => { | ||
| const proc = spawn("just", ["build-wasm-node-dev"], { | ||
| cwd: rootDir, | ||
| stdio: "pipe", | ||
| }); | ||
|
|
||
| let stderr = ""; | ||
|
|
||
| proc.stderr?.on("data", (data) => { | ||
| stderr += data.toString(); | ||
| }); | ||
|
|
||
| proc.on("close", (code) => { | ||
| if (code === 0) { | ||
| resolve(); | ||
| } else { | ||
| const error = new Error( | ||
| `WASM build failed with code ${code}:\n${stderr.trim()}`, | ||
| ); | ||
| reject(error); | ||
| } | ||
| }); | ||
|
|
||
| proc.on("error", (err) => { | ||
| const error = new Error(`Failed to start build process: ${err.message}`); | ||
| reject(error); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/prettier-compare/src/watch.ts | sed -n '233,259p'Repository: biomejs/biome
Length of output: 821
🏁 Script executed:
# Let's also check the full context of this function
cat -n packages/prettier-compare/src/watch.ts | sed -n '220,270p'Repository: biomejs/biome
Length of output: 1259
🏁 Script executed:
# Search for any stdout consumption in this file
rg "stdout" packages/prettier-compare/src/watch.ts -nRepository: biomejs/biome
Length of output: 39
Drain stdout to prevent back-pressure stalls in rebuildWasm.
Line 236 sets stdio: "pipe" but only consumes stderr. Buffered stdout will block the child process once the pipe fills, especially with verbose build output. Use ["ignore", "inherit", "pipe"] instead to let stdout stream through.
🛠️ Suggested fix
-const proc = spawn("just", ["build-wasm-node-dev"], {
- cwd: rootDir,
- stdio: "pipe",
-});
+const proc = spawn("just", ["build-wasm-node-dev"], {
+ cwd: rootDir,
+ stdio: ["ignore", "inherit", "pipe"],
+});🤖 Prompt for AI Agents
In `@packages/prettier-compare/src/watch.ts` around lines 233 - 259, In
rebuildWasm, the child spawn uses stdio: "pipe" but only consumes stderr, which
can cause stdout back-pressure; change the spawn options for the proc created in
rebuildWasm (the spawn call that runs "just", ["build-wasm-node-dev"]) to use
stdio: ["ignore", "inherit", "pipe"] so stdout is inherited (streams to parent)
while stderr remains piped and can still be collected by the existing
proc.stderr?.on("data") handler; leave the rest of the error/close handling
intact.
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
Adds a new tool for us to use to compare biome's and prettier's formatting IR and output. Instructions for how to use it are in it's README.md.
It's mostly AI generated, and probably could use a little cleanup.
TODO
Test Plan
Manually tested.
Docs