Support Webpack and fixes #558
Conversation
WalkthroughReplaces bundler-specific worker/worklet imports with a runtime loader (loadAudioWorklet) that temporarily patches navigator.serviceWorker.register; adds a Speaking subsystem to Audio, wires channelCount into AudioWorkletNode, updates video worker initialization, tweaks a UI label, and adds tooling/dependency changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Audio as Audio (publish/watch)
participant Hacks as loadAudioWorklet
participant Nav as navigator.serviceWorker
participant AW as AudioWorklet
participant Speaking as Speaking
App->>Audio: new Audio(props)
activate Audio
Audio->>Speaking: new Speaking(this, props?.speaking)
Note right of Speaking #DFF2E1: initialize speaking/VAD
Audio->>Nav: (temporarily patched) register -> returns module URL
Audio->>Hacks: call loadAudioWorklet(registerFn)
Hacks->>Nav: patch register, invoke registerFn, return URL, restore register
Hacks-->>Audio: module URL
Audio->>AW: audioContext.audioWorklet.addModule(module URL)
AW-->>Audio: module loaded
Audio->>Audio: new AudioWorkletNode(..., { channelCount: settings.channelCount })
Audio->>Audio: connect nodes, encoding, framing, catalog
deactivate Audio
sequenceDiagram
autonumber
actor App
participant Detect as Video Detection
participant W as Worker
App->>Detect: start()
Detect->>W: new Worker(new URL("./detection-worker", import.meta.url), { type: "module" })
Note over Detect,W #FFF4C2: Replaces Vite `?worker&url` import
Detect->>W: setup (Comlink, loop)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
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. ✨ Finishing Touches
🧪 Generate unit tests
🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
js/hang/src/publish/audio/index.ts (3)
186-196: Guard against undefined channelCount in encoder config to avoid runtime errors.
MediaStreamTrack.getSettings()may omitchannelCount. Passingundefinedintou53()will crash and also breaks bitrate derivation. Compute channels with a safe fallback.- const config = { + const channels = settings.channelCount ?? worklet.channelCount ?? 1; + const config = { // TODO get codec and description from decoderConfig codec: "opus", // Firefox doesn't provide the sampleRate in the settings. - sampleRate: u53(settings.sampleRate ?? worklet?.context.sampleRate), - numberOfChannels: u53(settings.channelCount), + sampleRate: u53(settings.sampleRate ?? worklet.context.sampleRate), + numberOfChannels: u53(channels), // TODO configurable - bitrate: u53(settings.channelCount * 32_000), + bitrate: u53(channels * 32_000), };
237-246: Slice input channels defensively when channelCount is missing.Avoid indexing with a possibly-undefined value.
- const channels = data.channels.slice(0, settings.channelCount); + const channelLimit = settings.channelCount ?? data.channels.length; + const channels = data.channels.slice(0, channelLimit);
282-286: Close Speaking on teardown to prevent leaks.
Speakingis instantiated but never closed; addthis.speaking.close()inclose().close() { this.#signals.close(); this.captions.close(); + this.speaking.close(); this.#track.close(); }
🧹 Nitpick comments (6)
js/package.json (1)
12-16: Prettier + Biome can conflict; pick one formatter or scope their responsibilitiesYou already use Biome, which includes a formatter. Adding Prettier may lead to churn and inconsistent diffs if both run over the same files.
Options:
- Prefer Biome for JS/TS/JSON; use Prettier only for markdown and non-Biome formats. Add explicit ignore patterns to avoid overlap.
- Or drop Prettier to keep a single-source-of-truth formatter.
Example scripts (if you keep both), scoping Prettier to markdown only:
{ "scripts": { "fmt": "biome format --write . && prettier --write \"**/*.{md,mdx}\"", "lint": "biome lint ." } }js/hang/src/watch/audio/index.ts (1)
95-103: Deduplicate the worklet-loading hack behind a small utilityThe same pattern appears in multiple files. Extract a helper, e.g.,
loadAudioWorklet(context: AudioContext, path: string), to centralize the guard/restore logic and avoid drift.I can draft a tiny
js/hang/src/util/worklet.tswith typedloadAudioWorklet()and update call sites on request.js/hang/src/publish/audio/captions.ts (1)
95-103: Consider extracting a shared loader helperThis is the same logic as in watch/audio. A shared
loadAudioWorklet()reduces risk of inconsistent fixes.Happy to add a small utility and update both call sites if you want.
js/hang/src/publish/audio/index.ts (3)
141-145: Honor explicit channel count on the AudioWorkletNode and provide a safe default.Without an explicit mode, the graph may up/down-mix implicitly. Also,
settings.channelCountcan be undefined on some browsers. Recommend settingchannelCountMode: "explicit"and defaulting to 1 when missing.- const worklet = new AudioWorkletNode(context, "capture", { - numberOfInputs: 1, - numberOfOutputs: 0, - channelCount: settings.channelCount, - }); + const worklet = new AudioWorkletNode(context, "capture", { + numberOfInputs: 1, + numberOfOutputs: 0, + // Ensure we honor a specific channel count instead of letting the graph pick one. + channelCountMode: "explicit", + channelCount: settings.channelCount ?? 1, + });
10-11: Consider re-exporting Speaking to mirror Captions ergonomics.If external callers need
SpeakingProps/Speakingtypes, a re-export keeps the public API consistent withcaptions.export * from "./captions"; +export * from "./speaking";
26-38: Typo: “vidfeo” → “video”.Very minor docstring fix.
-// This is a fork of the MediaTrackSettings interface with properties required for audio or vidfeo. +// This is a fork of the MediaTrackSettings interface with properties required for audio or video.
📜 Review details
Configuration used: CodeRabbit UI
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)
js/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
js/hang/src/publish/audio/captions.ts(1 hunks)js/hang/src/publish/audio/index.ts(2 hunks)js/hang/src/publish/video/detection.ts(1 hunks)js/hang/src/support/element.ts(1 hunks)js/hang/src/watch/audio/index.ts(1 hunks)js/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
js/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place JS/TS packages under the js/ directory
Files:
js/package.jsonjs/hang/src/publish/video/detection.tsjs/hang/src/watch/audio/index.tsjs/hang/src/support/element.tsjs/hang/src/publish/audio/captions.tsjs/hang/src/publish/audio/index.ts
{rs,js}/hang/**
📄 CodeRabbit inference engine (CLAUDE.md)
All media logic belongs in the hang layer (rs/hang and js/hang)
Files:
js/hang/src/publish/video/detection.tsjs/hang/src/watch/audio/index.tsjs/hang/src/support/element.tsjs/hang/src/publish/audio/captions.tsjs/hang/src/publish/audio/index.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). (1)
- GitHub Check: Check
🔇 Additional comments (6)
js/hang/src/support/element.ts (1)
303-307: Label change aligns audio decoding rows with encoding rows — looks goodSwitching the second-column label from “Audio” to “Opus” under Decoding improves specificity and matches the Encoding section’s per-codec layout.
js/hang/src/publish/video/detection.ts (1)
53-54: Worker init via new URL is bundler-agnostic—niceSwitching to
new Worker(new URL('./detection-worker', import.meta.url), { type: 'module' })should work across Webpack/Vite and fixes the Vite-only path. Cleanup viaterminate()is correct.Please verify in Next.js + Turbopack and legacy Webpack builds that:
- The worker chunk is emitted and reachable at runtime.
- No “Accessing import.meta directly is unsupported” warnings propagate from nested imports.
js/hang/src/publish/audio/index.ts (4)
86-95: Speaking integration looks good; initialize order is correct.Nice addition and consistent with
Captionsintegration. Make sure to pair it with the teardown fix above.
4-4: LGTM on integer helpers import adjustment.No functional impact; consistent with existing usage.
8-8: LGTM on Speaking import.Matches the new public
speakingsurface and catalog wiring.
132-140: EnsureserviceWorker.registeroverride is always restored and guard its absenceTo prevent permanently leaking an overridden
register(ifaddModulethrows) and avoid crashes in browsers without ServiceWorker support:
- File:
js/hang/src/publish/audio/index.ts
Location: inside theeffect.spawn(async () => { … })callback (around lines 130–140)Proposed patch:
- // Hacky workaround to support Webpack and Vite: - // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 - - const { register } = navigator.serviceWorker; - // @ts-ignore hack to make webpack believe that it is registering a worker - navigator.serviceWorker.register = (url: URL) => context.audioWorklet.addModule(url); - await navigator.serviceWorker.register(new URL("./capture-worklet", import.meta.url)); - navigator.serviceWorker.register = register; + // Hacky workaround to support Webpack and Vite: + // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 + // Guard against missing ServiceWorker and always restore the original register method. + const sw = (navigator as any).serviceWorker as ServiceWorkerContainer | undefined; + if (!sw || typeof sw.register !== "function") { + // If SW isn’t available, fall back to directly loading the worklet. + await context.audioWorklet.addModule(new URL("./capture-worklet", import.meta.url)); + } else { + const originalRegister = sw.register; + try { + // @ts-expect-error: Trick bundlers into treating the URL as a worker. + sw.register = (url: URL) => context.audioWorklet.addModule(url); + await sw.register(new URL("./capture-worklet", import.meta.url)); + } finally { + // Always restore—even if addModule rejects. + sw.register = originalRegister; + } + }Please verify this change in your build environments (Next.js/Webpack, Turbopack, Vite) and across browsers (Safari secure contexts and Chromium) to confirm there are no side-effects or silent failures.
| // Hacky workaround to support Webpack and Vite: | ||
| // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 | ||
|
|
||
| const { register } = navigator.serviceWorker; | ||
| // @ts-ignore hack to make webpack believe that it is registering a worker | ||
| navigator.serviceWorker.register = (url: URL) => ctx.audioWorklet.addModule(url); | ||
| await navigator.serviceWorker.register(new URL("./capture-worklet", import.meta.url)); | ||
| navigator.serviceWorker.register = register; | ||
|
|
There was a problem hiding this comment.
Same global override concerns: add guard + try/finally and prefer @ts-expect-error
Mirror the safer pattern here to prevent leaking the patched register and to handle no-SW environments gracefully.
Apply this diff:
- // Hacky workaround to support Webpack and Vite:
- // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214
-
- const { register } = navigator.serviceWorker;
- // @ts-ignore hack to make webpack believe that it is registering a worker
- navigator.serviceWorker.register = (url: URL) => ctx.audioWorklet.addModule(url);
- await navigator.serviceWorker.register(new URL("./capture-worklet", import.meta.url));
- navigator.serviceWorker.register = register;
+ // Hacky workaround to support Webpack and Vite:
+ // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214
+ const sw: any = (navigator as any).serviceWorker;
+ const workletUrl = new URL("./capture-worklet", import.meta.url);
+
+ if (!sw || typeof sw.register !== "function") {
+ await ctx.audioWorklet.addModule(workletUrl);
+ } else {
+ const originalRegister = sw.register.bind(sw);
+ // @ts-expect-error: Intentionally masquerading as ServiceWorker.register for bundlers.
+ sw.register = (url: URL) => ctx.audioWorklet.addModule(url);
+ try {
+ await sw.register(workletUrl);
+ } finally {
+ sw.register = originalRegister;
+ }
+ }📝 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.
| // Hacky workaround to support Webpack and Vite: | |
| // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 | |
| const { register } = navigator.serviceWorker; | |
| // @ts-ignore hack to make webpack believe that it is registering a worker | |
| navigator.serviceWorker.register = (url: URL) => ctx.audioWorklet.addModule(url); | |
| await navigator.serviceWorker.register(new URL("./capture-worklet", import.meta.url)); | |
| navigator.serviceWorker.register = register; | |
| // Hacky workaround to support Webpack and Vite: | |
| // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 | |
| const sw: any = (navigator as any).serviceWorker; | |
| const workletUrl = new URL("./capture-worklet", import.meta.url); | |
| if (!sw || typeof sw.register !== "function") { | |
| await ctx.audioWorklet.addModule(workletUrl); | |
| } else { | |
| const originalRegister = sw.register.bind(sw); | |
| // @ts-expect-error: Intentionally masquerading as ServiceWorker.register for bundlers. | |
| sw.register = (url: URL) => ctx.audioWorklet.addModule(url); | |
| try { | |
| await sw.register(workletUrl); | |
| } finally { | |
| sw.register = originalRegister; | |
| } | |
| } |
🤖 Prompt for AI Agents
In js/hang/src/publish/audio/captions.ts around lines 95 to 103, the code
overrides navigator.serviceWorker.register without a guard or try/finally and
uses @ts-ignore; change it to first check that navigator.serviceWorker and
navigator.serviceWorker.register exist, save the original register, replace it
with the worklet-adding function inside a try block, await registering the
worklet URL, and restore the original register in a finally block; also replace
the // @ts-ignore with // @ts-expect-error to indicate the intentional type
bypass and ensure the code no-ops gracefully when service workers are
unavailable.
js/hang/src/watch/audio/index.ts
Outdated
| // Hacky workaround to support Webpack and Vite: | ||
| // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 | ||
|
|
||
| const { register } = navigator.serviceWorker; | ||
| // @ts-ignore hack to make webpack believe that it is registering a worker | ||
| navigator.serviceWorker.register = (url: URL) => context.audioWorklet.addModule(url); | ||
| await navigator.serviceWorker.register(new URL("./render-worklet", import.meta.url)); | ||
| navigator.serviceWorker.register = register; | ||
|
|
There was a problem hiding this comment.
Guard and always-restore when monkey-patching navigator.serviceWorker.register
Overriding a global API without a guard and without a try/finally can leave the environment in a broken state if addModule throws. It will also throw on platforms where navigator.serviceWorker is undefined (HTTP, some Safari contexts).
Apply this diff to add a guard and guarantee restoration:
- // Hacky workaround to support Webpack and Vite:
- // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214
-
- const { register } = navigator.serviceWorker;
- // @ts-ignore hack to make webpack believe that it is registering a worker
- navigator.serviceWorker.register = (url: URL) => context.audioWorklet.addModule(url);
- await navigator.serviceWorker.register(new URL("./render-worklet", import.meta.url));
- navigator.serviceWorker.register = register;
+ // Hacky workaround to support Webpack and Vite:
+ // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214
+ const sw: any = (navigator as any).serviceWorker;
+ const workletUrl = new URL("./render-worklet", import.meta.url);
+
+ if (!sw || typeof sw.register !== "function") {
+ // Fallback: no Service Worker support—load directly.
+ await context.audioWorklet.addModule(workletUrl);
+ } else {
+ const originalRegister = sw.register.bind(sw);
+ // @ts-expect-error: Intentionally masquerading as ServiceWorker.register for bundlers.
+ sw.register = (url: URL) => context.audioWorklet.addModule(url);
+ try {
+ await sw.register(workletUrl);
+ } finally {
+ sw.register = originalRegister;
+ }
+ }📝 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.
| // Hacky workaround to support Webpack and Vite: | |
| // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 | |
| const { register } = navigator.serviceWorker; | |
| // @ts-ignore hack to make webpack believe that it is registering a worker | |
| navigator.serviceWorker.register = (url: URL) => context.audioWorklet.addModule(url); | |
| await navigator.serviceWorker.register(new URL("./render-worklet", import.meta.url)); | |
| navigator.serviceWorker.register = register; | |
| // Hacky workaround to support Webpack and Vite: | |
| // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 | |
| const sw: any = (navigator as any).serviceWorker; | |
| const workletUrl = new URL("./render-worklet", import.meta.url); | |
| if (!sw || typeof sw.register !== "function") { | |
| // Fallback: no Service Worker support—load directly. | |
| await context.audioWorklet.addModule(workletUrl); | |
| } else { | |
| const originalRegister = sw.register.bind(sw); | |
| // @ts-expect-error: Intentionally masquerading as ServiceWorker.register for bundlers. | |
| sw.register = (url: URL) => context.audioWorklet.addModule(url); | |
| try { | |
| await sw.register(workletUrl); | |
| } finally { | |
| sw.register = originalRegister; | |
| } | |
| } |
🤖 Prompt for AI Agents
In js/hang/src/watch/audio/index.ts around lines 95 to 103, the code
monkey-patches navigator.serviceWorker.register without checking
navigator.serviceWorker exists and without a try/finally to guarantee
restoration; change it to first guard that navigator.serviceWorker and
navigator.serviceWorker.register are defined, save the original register, then
perform the temporary assignment and await addModule inside a try block, and
always restore the original register in a finally block so the global is never
left patched even if addModule throws or navigator.serviceWorker is absent.
|
Hopefully it works with Vite. Worklets are awful. |
|
And yeah, the code rabbit review points out this could be racey. Maybe add a |
|
also |
But my vscode formats on save and by default uses a different prettier version which causes a bunch of changes. If the prettier version is fixed in the project, it will always use that version -> no changes. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
js/hang/src/publish/audio/speaking.ts (2)
87-95: Serialize monkey-patching to prevent races across async callers.Two concurrent calls could overlap while
registeris patched. Consider a small module-scoped mutex and a shared helper so all worklet loaders use the same critical section. This also consolidates the hack in one place.Example helper (new file suggestion):
// js/hang/src/worklet/load.ts type Loader = (url: URL) => Promise<void>; class Mutex { #p: Promise<void> = Promise.resolve(); async run<T>(fn: () => Promise<T>): Promise<T> { const prev = this.#p; let release!: () => void; this.#p = new Promise<void>((r) => (release = r)); await prev; try { return await fn(); } finally { release(); } } } const patchMutex = new Mutex(); export async function loadAudioWorkletModule(ctx: AudioContext, moduleUrl: URL): Promise<void> { return patchMutex.run(async () => { const sw = navigator.serviceWorker; if (sw?.register) { const original = sw.register; // @ts-expect-error spoof for bundlers sw.register = ((url: string | URL, ..._rest: unknown[]) => ctx.audioWorklet.addModule(typeof url === "string" ? new URL(url, import.meta.url) : url) ) as typeof sw.register; try { await sw.register(moduleUrl); } finally { sw.register = original; } } else { await ctx.audioWorklet.addModule(moduleUrl); } }); }Then here replace the patched block with:
await loadAudioWorkletModule(ctx, new URL("./capture-worklet", import.meta.url));I can push a follow-up patch touching all similar sites (captions, watch/audio render worklet) to use this helper.
91-91: Prefer @ts-expect-error over @ts-ignore.
@ts-expect-errordocuments the intentional violation and will fail the build if/when it’s no longer needed.
📜 Review details
Configuration used: CodeRabbit UI
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 selected for processing (1)
js/hang/src/publish/audio/speaking.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{rs,js}/hang/**
📄 CodeRabbit inference engine (CLAUDE.md)
All media logic belongs in the hang layer (rs/hang and js/hang)
Files:
js/hang/src/publish/audio/speaking.ts
js/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place JS/TS packages under the js/ directory
Files:
js/hang/src/publish/audio/speaking.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). (1)
- GitHub Check: Check
| // Hacky workaround to support Webpack and Vite: | ||
| // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 | ||
|
|
||
| const { register } = navigator.serviceWorker; | ||
| // @ts-ignore hack to make webpack believe that it is registering a worker | ||
| navigator.serviceWorker.register = (url: URL) => ctx.audioWorklet.addModule(url); | ||
| await navigator.serviceWorker.register(new URL("./capture-worklet", import.meta.url)); | ||
| navigator.serviceWorker.register = register; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Double-check Webpack/Next dev servers where ServiceWorker may be disabled.
On non-HTTPS origins (except localhost) or certain Next.js dev configs, navigator.serviceWorker can be unavailable. The fallback path in the proposed diff handles this; please verify in your target environments.
Run to find all monkey-patch sites and ensure they either use the helper or include the same fallback + try/finally:
🏁 Script executed:
#!/bin/bash
# Look for monkey-patching and worklet loads across the repo
rg -nC2 -g 'js/**' -e 'serviceWorker\.register\s*=' -e 'new URL\(".*worklet", import\.meta\.url\)' -e 'audioWorklet\.addModule'Length of output: 2802
Ensure all ServiceWorker monkey-patches include fallback handling and are wrapped in try/finally
We’ve identified four spots in the codebase where navigator.serviceWorker.register is temporarily overridden to load an AudioWorklet module. Each of these must:
- Check for the existence of
navigator.serviceWorker(to cover non-HTTPS or dev-server scenarios where it’s disabled). - Wrap the patch/revert sequence in a
try { … } finally { … }block to guarantee restoration of the originalregistermethod, even ifaddModulethrows.
Affected files:
js/hang/src/watch/audio/index.ts(lines 98–103)js/hang/src/publish/audio/speaking.ts(lines 90–95)js/hang/src/publish/audio/index.ts(lines 135–140)js/hang/src/publish/audio/captions.ts(lines 98–103)
Please refactor each of these blocks to something like:
if (navigator.serviceWorker) {
const originalRegister = navigator.serviceWorker.register;
// @ts-ignore allow hack for bundlers
navigator.serviceWorker.register = (url: URL) => ctx.audioWorklet.addModule(url);
try {
await navigator.serviceWorker.register(new URL("./…-worklet", import.meta.url));
} finally {
navigator.serviceWorker.register = originalRegister;
}
} else {
// fallback: directly load the worklet
await ctx.audioWorklet.addModule(new URL("./…-worklet", import.meta.url));
}This ensures both robust cleanup and support on origins without ServiceWorker.
🤖 Prompt for AI Agents
In js/hang/src/publish/audio/speaking.ts around lines 87 to 95, the temporary
override of navigator.serviceWorker.register lacks existence checks, guaranteed
restoration, and a fallback; update the block to first check if
navigator.serviceWorker exists, save the original register, override it (keeping
the @ts-ignore comment), then call the patched register inside a try { await ...
} finally { restore original register } so the original is always restored even
if addModule throws; if navigator.serviceWorker is not present, call
ctx.audioWorklet.addModule(...) directly as the fallback.
🛠️ Refactor suggestion
Ensure global patch is always reverted (try/finally) + add SW fallback + avoid destructuring trap.
If ctx.audioWorklet.addModule() throws, navigator.serviceWorker.register stays monkey‑patched, potentially breaking unrelated code. Also, destructuring register isn’t needed here and can be avoided. Add a guarded patch with a fallback when serviceWorker is unavailable, preserve the call signature, and always restore via try/finally.
Apply this diff:
@@
- const { register } = navigator.serviceWorker;
- // @ts-ignore hack to make webpack believe that it is registering a worker
- navigator.serviceWorker.register = (url: URL) => ctx.audioWorklet.addModule(url);
- await navigator.serviceWorker.register(new URL("./capture-worklet", import.meta.url));
- navigator.serviceWorker.register = register;
+ const sw = navigator.serviceWorker;
+ const moduleUrl = new URL("./capture-worklet", import.meta.url);
+ if (sw?.register) {
+ const originalRegister = sw.register;
+ // @ts-expect-error We intentionally spoof the signature to trick bundlers.
+ sw.register = ((url: string | URL, ..._rest: unknown[]) =>
+ ctx.audioWorklet.addModule(typeof url === "string" ? new URL(url, import.meta.url) : url)
+ ) as typeof sw.register;
+ try {
+ await sw.register(moduleUrl);
+ } finally {
+ sw.register = originalRegister;
+ }
+ } else {
+ // Fallback: when ServiceWorker is unavailable (e.g., some environments), load directly.
+ await ctx.audioWorklet.addModule(moduleUrl);
+ }📝 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.
| // Hacky workaround to support Webpack and Vite: | |
| // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 | |
| const { register } = navigator.serviceWorker; | |
| // @ts-ignore hack to make webpack believe that it is registering a worker | |
| navigator.serviceWorker.register = (url: URL) => ctx.audioWorklet.addModule(url); | |
| await navigator.serviceWorker.register(new URL("./capture-worklet", import.meta.url)); | |
| navigator.serviceWorker.register = register; | |
| // Hacky workaround to support Webpack and Vite: | |
| // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 | |
| const sw = navigator.serviceWorker; | |
| const moduleUrl = new URL("./capture-worklet", import.meta.url); | |
| if (sw?.register) { | |
| const originalRegister = sw.register; | |
| // @ts-expect-error We intentionally spoof the signature to trick bundlers. | |
| sw.register = ((url: string | URL, ..._rest: unknown[]) => | |
| ctx.audioWorklet.addModule( | |
| typeof url === "string" ? new URL(url, import.meta.url) : url | |
| ) | |
| ) as typeof sw.register; | |
| try { | |
| await sw.register(moduleUrl); | |
| } finally { | |
| sw.register = originalRegister; | |
| } | |
| } else { | |
| // Fallback: when ServiceWorker is unavailable (e.g., some environments), load directly. | |
| await ctx.audioWorklet.addModule(moduleUrl); | |
| } |
🤖 Prompt for AI Agents
In js/hang/src/publish/audio/speaking.ts around lines 87–95, avoid destructuring
and ensure the global monkey-patch is always reverted and supports missing
serviceWorker: check if navigator.serviceWorker exists, save the original
register function (if any) into a variable (no destructuring), replace
navigator.serviceWorker.register with a function that preserves the original
call signature and delegates to ctx.audioWorklet.addModule(url) as the
workaround, perform the registration inside a try block and restore the original
register in a finally block so the global is always reverted, and if
navigator.serviceWorker is absent provide a safe fallback that directly calls
ctx.audioWorklet.addModule or rejects appropriately.
Yeah I'm using biome, not prettier. It's faster and IMO does a better job at formatting. |
|
yolo I'll let you know if it breaks stuff for me. |
|
👍 🤞 I did a quick test in Vite and it looked good. But I have not tested all the caption stuff. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
js/hang/src/watch/audio/index.ts (1)
100-119: Minor: keep channelCount in sync with worklet init and validate at runtimeYou derive
channelCountearlier (Line 81) and pass it in the init message. Consider also validating the created node’schannelCountmatches expectations and logging once if not, to help diagnose platform-specific defaults.Example (no diff necessary):
- After constructing the node, compare
worklet.channelCountvs thechannelCountyou computed and warn if they differ.js/hang/src/publish/audio/index.ts (1)
139-144: Channel count may be undefined on some browsers; provide a sane defaultSome implementations omit
channelCountinMediaTrackSettings. Passingundefinedcan produce surprising defaults. Consider defaulting to2(stereo) or probing from theMediaStreamTrackcapabilities.Apply:
- const worklet = new AudioWorkletNode(context, "capture", { + const worklet = new AudioWorkletNode(context, "capture", { numberOfInputs: 1, numberOfOutputs: 0, - channelCount: settings.channelCount, + channelCount: settings.channelCount ?? 2, });If you prefer stricter behavior, throw when
channelCountis undefined in dev builds to catch device quirks early.
📜 Review details
Configuration used: CodeRabbit UI
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)
js/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
js/hang/package.json(1 hunks)js/hang/src/publish/audio/captions.ts(2 hunks)js/hang/src/publish/audio/index.ts(2 hunks)js/hang/src/publish/audio/speaking.ts(2 hunks)js/hang/src/util/hacks.ts(1 hunks)js/hang/src/watch/audio/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- js/hang/src/publish/audio/speaking.ts
- js/hang/src/publish/audio/captions.ts
🧰 Additional context used
📓 Path-based instructions (2)
{rs,js}/hang/**
📄 CodeRabbit inference engine (CLAUDE.md)
All media logic belongs in the hang layer (rs/hang and js/hang)
Files:
js/hang/src/util/hacks.tsjs/hang/package.jsonjs/hang/src/publish/audio/index.tsjs/hang/src/watch/audio/index.ts
js/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place JS/TS packages under the js/ directory
Files:
js/hang/src/util/hacks.tsjs/hang/package.jsonjs/hang/src/publish/audio/index.tsjs/hang/src/watch/audio/index.ts
🧬 Code graph analysis (2)
js/hang/src/publish/audio/index.ts (1)
js/hang/src/util/hacks.ts (1)
loadAudioWorklet(16-29)
js/hang/src/watch/audio/index.ts (1)
js/hang/src/util/hacks.ts (1)
loadAudioWorklet(16-29)
🔇 Additional comments (3)
js/hang/package.json (1)
41-47: async-mutex ESM/CJS interop verifiedI’ve confirmed that [email protected] ships proper named exports for both ESM and CJS consumers:
- The package.json
exportsfield maps
•"import"→./index.mjs(ESM)
•"require"→./lib/index.js(CJS)- The root ESM entrypoint (
index.mjs) ends with
export { E_ALREADY_LOCKED, E_CANCELED, E_TIMEOUT, Mutex, Semaphore, tryAcquire, withTimeout };- The secondary ES build (
es6/index.js) re-exportsMutexas a named export:
export { default as Mutex } from './Mutex';Imports in the codebase (found in
js/hang/src/util/hacks.tsandjs/moq/src/ietf/control.ts) useimport { Mutex } from "async-mutex";which will correctly resolve in Node ESM (Next.js SSR), Vite, Webpack, and plain CJS contexts.
No changes needed here—named imports of
Mutexare safe across all targeted environments. Approved.js/hang/src/util/hacks.ts (1)
14-29: Mutex-based critical section is the right directionSerializing the monkey-patch with a module-scoped Mutex eliminates the race flagged in the PR discussion and guarantees restoration with the
finallyblock. Once the guard above is added, this becomes robust across async re-entrancy.js/hang/src/publish/audio/index.ts (1)
186-257: Nice: end-to-end framing logic remains unchanged and Speaking integration is orthogonalThe encoder path and catalog updates remain stable; the Speaking addition fits cleanly without altering existing behavior.
| await context.audioWorklet.addModule( | ||
| await loadAudioWorklet(() => | ||
| navigator.serviceWorker.register(new URL("./capture-worklet", import.meta.url)), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Adopt the guarded loadAudioWorklet(url, registerFn) and compute URL once
Prevents crashes on platforms without Service Worker support while preserving the bundler hint.
Apply:
- await context.audioWorklet.addModule(
- await loadAudioWorklet(() =>
- navigator.serviceWorker.register(new URL("./capture-worklet", import.meta.url)),
- ),
- );
+ const workletUrl = new URL("./capture-worklet", import.meta.url);
+ await context.audioWorklet.addModule(
+ await loadAudioWorklet(workletUrl, () => navigator.serviceWorker.register(workletUrl)),
+ );📝 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.
| await context.audioWorklet.addModule( | |
| await loadAudioWorklet(() => | |
| navigator.serviceWorker.register(new URL("./capture-worklet", import.meta.url)), | |
| ), | |
| ); | |
| // Compute the worklet URL once for bundler hints and reuse | |
| const workletUrl = new URL("./capture-worklet", import.meta.url); | |
| await context.audioWorklet.addModule( | |
| await loadAudioWorklet( | |
| workletUrl, | |
| () => navigator.serviceWorker.register(workletUrl), | |
| ), | |
| ); |
🤖 Prompt for AI Agents
In js/hang/src/publish/audio/index.ts around lines 133-137, the current call
inlines navigator.serviceWorker.register inside loadAudioWorklet which can crash
on platforms without Service Worker support; compute the worklet URL once into a
const (e.g. const workletUrl = new URL("./capture-worklet", import.meta.url))
and then call the guarded loadAudioWorklet(workletUrl, () =>
navigator.serviceWorker?.register(workletUrl)) so the helper can check for
service worker support before invoking the register function and the bundler
still sees the URL hint.
| // https://issues.chromium.org/issues/40504498 | ||
| export const isChrome = navigator.userAgent.toLowerCase().includes("chrome"); | ||
|
|
||
| // https://bugzilla.mozilla.org/show_bug.cgi?id=1967793 | ||
| export const isFirefox = navigator.userAgent.toLowerCase().includes("firefox"); | ||
|
|
There was a problem hiding this comment.
Top-level navigator access will crash in SSR and non-browser contexts
Accessing navigator at module load time throws in SSR (Next.js) and workers without Navigator. Gate userAgent reads.
Apply:
-// https://issues.chromium.org/issues/40504498
-export const isChrome = navigator.userAgent.toLowerCase().includes("chrome");
-
-// https://bugzilla.mozilla.org/show_bug.cgi?id=1967793
-export const isFirefox = navigator.userAgent.toLowerCase().includes("firefox");
+// https://issues.chromium.org/issues/40504498
+const ua = typeof navigator !== "undefined" && typeof navigator.userAgent === "string"
+ ? navigator.userAgent.toLowerCase()
+ : "";
+export const isChrome = ua.includes("chrome");
+
+// https://bugzilla.mozilla.org/show_bug.cgi?id=1967793
+export const isFirefox = ua.includes("firefox");📝 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.
| // https://issues.chromium.org/issues/40504498 | |
| export const isChrome = navigator.userAgent.toLowerCase().includes("chrome"); | |
| // https://bugzilla.mozilla.org/show_bug.cgi?id=1967793 | |
| export const isFirefox = navigator.userAgent.toLowerCase().includes("firefox"); | |
| // https://issues.chromium.org/issues/40504498 | |
| const ua = typeof navigator !== "undefined" && typeof navigator.userAgent === "string" | |
| ? navigator.userAgent.toLowerCase() | |
| : ""; | |
| export const isChrome = ua.includes("chrome"); | |
| // https://bugzilla.mozilla.org/show_bug.cgi?id=1967793 | |
| export const isFirefox = ua.includes("firefox"); |
🤖 Prompt for AI Agents
In js/hang/src/util/hacks.ts around lines 3 to 8, the module reads
navigator.userAgent at top-level which will throw in SSR or non-browser
contexts; change the exports to safe, lazily-evaluated checks that first ensure
navigator (or globalThis.navigator) exists and that navigator.userAgent is a
string (e.g., typeof globalThis !== "undefined" && typeof globalThis.navigator
!== "undefined" && typeof globalThis.navigator.userAgent === "string") before
calling toLowerCase().includes(...), or export functions/getters (isChrome(),
isFirefox()) that perform this guarded check at call time so the module load
never accesses navigator in non-browser environments.
| export async function loadAudioWorklet(registerFn: () => Promise<ServiceWorkerRegistration>) { | ||
| return await loadAudioWorkletMutex.runExclusive(async () => { | ||
| const { register } = navigator.serviceWorker; | ||
|
|
||
| // @ts-ignore hack to make webpack believe that it is registering a worker | ||
| navigator.serviceWorker.register = (url: URL) => Promise.resolve(url); | ||
|
|
||
| try { | ||
| return (await registerFn()) as unknown as URL; | ||
| } finally { | ||
| navigator.serviceWorker.register = register; | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard serviceWorker presence; avoid destructuring undefined and provide a safe fallback
As written, const { register } = navigator.serviceWorker; throws when serviceWorker is undefined (HTTP, some Safari, certain embedded webviews), and calling registerFn() will then also fail. We need a fallback that still satisfies bundlers but doesn’t execute a broken call when SW is absent. Also prefer @ts-expect-error over blanket @ts-ignore.
Proposed minimal API change: accept the worklet URL upfront and keep the current registerFn for bundler visibility. If SW is missing, return the URL without evaluating registerFn. This also solves SSR cases where navigator exists at runtime but serviceWorker does not.
Apply:
-const loadAudioWorkletMutex = new Mutex();
-
-export async function loadAudioWorklet(registerFn: () => Promise<ServiceWorkerRegistration>) {
- return await loadAudioWorkletMutex.runExclusive(async () => {
- const { register } = navigator.serviceWorker;
-
- // @ts-ignore hack to make webpack believe that it is registering a worker
- navigator.serviceWorker.register = (url: URL) => Promise.resolve(url);
-
- try {
- return (await registerFn()) as unknown as URL;
- } finally {
- navigator.serviceWorker.register = register;
- }
- });
-}
+const loadAudioWorkletMutex = new Mutex();
+
+export async function loadAudioWorklet(
+ url: URL,
+ registerFn?: () => Promise<ServiceWorkerRegistration>,
+): Promise<URL> {
+ return await loadAudioWorkletMutex.runExclusive(async () => {
+ const nav: any = typeof navigator !== "undefined" ? navigator : undefined;
+ const sw: any = nav?.serviceWorker;
+
+ // If no Service Worker environment or no register shim is provided, use the URL directly.
+ if (!sw || typeof sw.register !== "function" || !registerFn) {
+ return url;
+ }
+
+ const originalRegister = sw.register.bind(sw);
+ // Intentionally masquerade as ServiceWorkerContainer.register for bundlers; returns the URL.
+ // The call still exists in user code so bundlers can statically detect it.
+ // @ts-expect-error: return type intentionally diverges at runtime
+ sw.register = (_url: URL) => Promise.resolve(url);
+ try {
+ await registerFn();
+ return url;
+ } finally {
+ sw.register = originalRegister;
+ }
+ });
+}Follow-up: this requires small call-site edits (see comments in watch/publish files). I can push a commit if you’d like.
📝 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 async function loadAudioWorklet(registerFn: () => Promise<ServiceWorkerRegistration>) { | |
| return await loadAudioWorkletMutex.runExclusive(async () => { | |
| const { register } = navigator.serviceWorker; | |
| // @ts-ignore hack to make webpack believe that it is registering a worker | |
| navigator.serviceWorker.register = (url: URL) => Promise.resolve(url); | |
| try { | |
| return (await registerFn()) as unknown as URL; | |
| } finally { | |
| navigator.serviceWorker.register = register; | |
| } | |
| }); | |
| } | |
| const loadAudioWorkletMutex = new Mutex(); | |
| export async function loadAudioWorklet( | |
| url: URL, | |
| registerFn?: () => Promise<ServiceWorkerRegistration>, | |
| ): Promise<URL> { | |
| return await loadAudioWorkletMutex.runExclusive(async () => { | |
| const nav: any = typeof navigator !== "undefined" ? navigator : undefined; | |
| const sw: any = nav?.serviceWorker; | |
| // If no Service Worker environment or no register shim is provided, use the URL directly. | |
| if (!sw || typeof sw.register !== "function" || !registerFn) { | |
| return url; | |
| } | |
| const originalRegister = sw.register.bind(sw); | |
| // Intentionally masquerade as ServiceWorkerContainer.register for bundlers; returns the URL. | |
| // The call still exists in user code so bundlers can statically detect it. | |
| // @ts-expect-error: return type intentionally diverges at runtime | |
| sw.register = (_url: URL) => Promise.resolve(url); | |
| try { | |
| await registerFn(); | |
| return url; | |
| } finally { | |
| sw.register = originalRegister; | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In js/hang/src/util/hacks.ts around lines 16–29, the code destructures
navigator.serviceWorker and calls registerFn unguarded which throws when
serviceWorker is undefined; change the function signature to accept the worklet
URL upfront (preserve registerFn for bundler visibility), check if
navigator?.serviceWorker exists before touching or destructuring it, and if
absent immediately return the provided URL (avoid invoking registerFn); when
stubbing navigator.serviceWorker.register use @ts-expect-error instead of
@ts-ignore, and ensure the original register property is restored in finally.
| // Register the AudioWorklet processor | ||
| await context.audioWorklet.addModule(RenderWorklet); | ||
| await context.audioWorklet.addModule( | ||
| await loadAudioWorklet(() => | ||
| navigator.serviceWorker.register(new URL("./render-worklet", import.meta.url)), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use the safer loadAudioWorklet(url, registerFn) form and avoid duplicate URL construction
This avoids executing a register call when Service Workers are unavailable and keeps the bundler-visible pattern. It also reduces duplication.
Apply:
- // Register the AudioWorklet processor
- await context.audioWorklet.addModule(
- await loadAudioWorklet(() =>
- navigator.serviceWorker.register(new URL("./render-worklet", import.meta.url)),
- ),
- );
+ // Register the AudioWorklet processor
+ const workletUrl = new URL("./render-worklet", import.meta.url);
+ await context.audioWorklet.addModule(
+ await loadAudioWorklet(workletUrl, () => navigator.serviceWorker.register(workletUrl)),
+ );📝 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.
| // Register the AudioWorklet processor | |
| await context.audioWorklet.addModule(RenderWorklet); | |
| await context.audioWorklet.addModule( | |
| await loadAudioWorklet(() => | |
| navigator.serviceWorker.register(new URL("./render-worklet", import.meta.url)), | |
| ), | |
| ); | |
| // Register the AudioWorklet processor | |
| const workletUrl = new URL("./render-worklet", import.meta.url); | |
| await context.audioWorklet.addModule( | |
| await loadAudioWorklet(workletUrl, () => navigator.serviceWorker.register(workletUrl)), | |
| ); |
🤖 Prompt for AI Agents
In js/hang/src/watch/audio/index.ts around lines 93–98, replace the inline
register-call form with the safer loadAudioWorklet(url, registerFn) pattern:
construct the worklet module URL once (e.g. new URL("./render-worklet",
import.meta.url) stored in a variable) and pass that URL as the first argument
to loadAudioWorklet and a register function that calls
navigator.serviceWorker.register with that same URL as the second argument; this
avoids executing registration when Service Workers are unavailable and
eliminates duplicate URL construction.
|
@skirsten Unfortunately this seems to break vite in production (local dev is fine). I'm going to revert for now if that's okay while we figure it out. |
This reverts commit 9c42e0d.
|
And for the record, dealing with AudioWorklet, AudioWorker, and WASM in bundlers is easily in my top 5 most despised things. I've lost days trying to get bundlers to work the same way. |
|
What is the issue you are seeing? I just quickly ran and it seems fine (apart from only building the index.html file), but the assets for the workers are there. |
|
Hmm, nevermind that does not look right... |
2 participants
On Next.js there is still some problems with the transformers library being loaded as node module somehow...
but its not blocking anything.
Also turbopack just gives up without any error message and return code 0 😄
Summary by CodeRabbit
New Features
Bug Fixes
Chores