fix(playground): make svelte work for prettier#3697
Conversation
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Walkthroughpackage.json adds Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 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). (4)
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: 0
🧹 Nitpick comments (1)
src/playground/PlaygroundLoader.tsx (1)
95-117: Log handlers implemented correctly.The message handling for cross-context logging from both workers is sound and integrates well with the worker-side logging bridges.
The log handling logic is duplicated between the two workers. Consider extracting a helper function to reduce duplication:
🔎 Optional refactor to reduce duplication
+function createLogHandler(workerName: string) { + return (event: MessageEvent) => { + const { level, message } = event.data as { + level: "log" | "info" | "warn" | "error"; + message: unknown[]; + }; + const prefix = `[${workerName}]`; + switch (level) { + case "log": + console.log(prefix, ...message); + break; + case "info": + console.info(prefix, ...message); + break; + case "warn": + console.warn(prefix, ...message); + break; + case "error": + console.error(prefix, ...message); + break; + default: + console.log(prefix, ...message); + } + }; +} + workerRef.current.addEventListener("message", (event) => { switch (event.data.type) { // ... existing cases ... case "log": { - const { level, message } = event.data as { - level: "log" | "info" | "warn" | "error"; - message: unknown[]; - }; - switch (level) { - case "log": - console.log("[Biome worker]", ...message); - break; - case "info": - console.info("[Biome worker]", ...message); - break; - case "warn": - console.warn("[Biome worker]", ...message); - break; - case "error": - console.error("[Biome worker]", ...message); - break; - default: - console.log("[Biome worker]", ...message); - } + createLogHandler("Biome worker")(event); break; }Also applies to: 141-163
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.json(2 hunks)src/playground/PlaygroundLoader.tsx(2 hunks)src/playground/workers/biomeWorker.ts(1 hunks)src/playground/workers/prettierWorker.ts(4 hunks)
🧰 Additional context used
🪛 GitHub Actions: Integrate
src/playground/workers/biomeWorker.ts
[error] 1-1: Cannot find module '@biomejs/wasm-web' or its corresponding type declarations.
src/playground/PlaygroundLoader.tsx
[error] 5-5: Cannot find module '@biomejs/wasm-web' or its corresponding type declarations.
src/playground/workers/prettierWorker.ts
[error] 1-1: Cannot find module '@biomejs/wasm-web' or its corresponding type declarations.
🪛 GitHub Actions: Playwright Tests
src/playground/PlaygroundLoader.tsx
[error] 1-1: Build failed: Failed to resolve entry for package '@biomejs/wasm-web'. The package may have incorrect main/module/exports specified in its package.json.
🔇 Additional comments (5)
src/playground/workers/biomeWorker.ts (1)
37-77: Excellent console bridging implementation.The logging bridge properly captures console output, serialises it defensively (with JSON.stringify and String() fallback), and forwards structured messages to the main thread whilst preserving the original console behaviour.
src/playground/workers/prettierWorker.ts (3)
40-80: Console bridging matches biomeWorker implementation.The logging bridge is implemented consistently with the Biome worker, which is excellent for maintainability.
193-195: Key fix for Svelte formatting.Changing
embeddedLanguageFormattingto"auto"and addingsvelteIndentScriptAndStyleare the crucial changes that enable proper Svelte formatting. Well done!
2-2: Imports are already correct for Prettier 3.7.4.The import path
"prettier/plugins/babel"aligns with the Prettier 3.0+ migration, which replaced the earlier"prettier/parser-babel"naming convention. Prettier 3.0 introducedstandalone.mjsin place of the earlieresm/standalone.mjs, and the correct Node.js import for plugins isprettier/plugins/graphql(and similarly for other plugins). The current imports require no changes.package.json (1)
68-68: Dependency updates sound solid.The prettier-plugin-svelte patch to 3.4.1 (latest version) and addition of svelte 5.46.0 (latest version) align with the PR's goal to fix Svelte formatting support. No security advisories apply to these versions.
| "starlight-blog": "0.25.2", | ||
| "starlight-changelogs": "0.2.3", | ||
| "starlight-links-validator": "0.18.0", | ||
| "svelte": "^5.46.0", |
There was a problem hiding this comment.
needed because the plugin uses the svelte compiler to parse svelte
| embeddedLanguageFormatting: "off", | ||
| embeddedLanguageFormatting: "auto", | ||
| vueIndentScriptAndStyle: options.vueIndentScriptAndStyle, | ||
| svelteIndentScriptAndStyle: options.vueIndentScriptAndStyle, |
There was a problem hiding this comment.
svelte has it's own option for this
dyc3
force-pushed
the
dyc3/fix-prettier-svelte
branch
from
6e21027 to
1b50f67
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/playground/workers/biomeWorker.ts (1)
62-77: LGTM! Consider a helper to reduce duplication.The console overrides work correctly but follow a repetitive pattern. A small helper could tidy this up if you're ever back here.
Optional refactor to reduce duplication
+type LogLevel = "log" | "info" | "warn" | "error"; + +function wrapConsoleMethod(level: LogLevel) { + const original = originalConsole[level]; + console[level] = (...args: unknown[]) => { + postLog(level, args); + original(...args); + }; +} + -console.log = (...args: unknown[]) => { - postLog("log", args); - originalConsole.log(...args); -}; -console.info = (...args: unknown[]) => { - postLog("info", args); - originalConsole.info(...args); -}; -console.warn = (...args: unknown[]) => { - postLog("warn", args); - originalConsole.warn(...args); -}; -console.error = (...args: unknown[]) => { - postLog("error", args); - originalConsole.error(...args); -}; +(["log", "info", "warn", "error"] as const).forEach(wrapConsoleMethod);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.jsonsrc/playground/PlaygroundLoader.tsxsrc/playground/workers/biomeWorker.tssrc/playground/workers/prettierWorker.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- src/playground/PlaygroundLoader.tsx
- src/playground/workers/prettierWorker.ts
⏰ 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). (3)
- GitHub Check: Redirect rules - biomejs
- GitHub Check: Header rules - biomejs
- GitHub Check: Pages changed - biomejs
🔇 Additional comments (2)
src/playground/workers/biomeWorker.ts (2)
37-42: LGTM! Preserving original console methods.Good practice to stash the originals before wrapping.
44-60: LGTM! Robust serialisation with appropriate error handling.The nested try-catch structure ensures logging failures never break the worker—exactly what you want from infrastructure code.
dyc3
force-pushed
the
dyc3/fix-prettier-svelte
branch
from
1b50f67 to
e2bffcc
Compare
2 participants
Summary
I finally figured out why the svelte prettier plugin wasn't working. This PR fixes it, and makes it a bit easier to get log output from the workers.