-
Notifications
You must be signed in to change notification settings - Fork 135
Support Webpack and fixes #558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
93c8945
1daa163
2f2fe19
240ff96
3e8eac6
2474fb4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,29 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Mutex } from "async-mutex"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
3
to
+8
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Hacky workaround to support Webpack and Vite | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // https://github.com/webpack/webpack/issues/11543#issuecomment-2045809214 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Note that Webpack needs to see `navigator.serviceWorker.register(new URL("literal"), ...)` for this to work | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+16
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Guard serviceWorker presence; avoid destructuring undefined and provide a safe fallback As written, 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 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,14 +2,14 @@ import type * as Moq from "@kixelated/moq"; | |||||||||||||||||||||||||
| import { Effect, type Getter, Signal } from "@kixelated/signals"; | ||||||||||||||||||||||||||
| import type * as Catalog from "../../catalog"; | ||||||||||||||||||||||||||
| import * as Container from "../../container"; | ||||||||||||||||||||||||||
| import { loadAudioWorklet } from "../../util/hacks"; | ||||||||||||||||||||||||||
| import * as Hex from "../../util/hex"; | ||||||||||||||||||||||||||
| import { Captions, type CaptionsProps } from "./captions"; | ||||||||||||||||||||||||||
| import type * as Render from "./render"; | ||||||||||||||||||||||||||
| import { Speaking, type SpeakingProps } from "./speaking"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export * from "./emitter"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import { Captions, type CaptionsProps } from "./captions"; | ||||||||||||||||||||||||||
| import { Speaking, type SpeakingProps } from "./speaking"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export type AudioProps = { | ||||||||||||||||||||||||||
| // Enable to download the audio track. | ||||||||||||||||||||||||||
| enabled?: boolean; | ||||||||||||||||||||||||||
|
|
@@ -24,9 +24,6 @@ export type AudioProps = { | |||||||||||||||||||||||||
| speaking?: SpeakingProps; | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Unfortunately, we need to use a Vite-exclusive import for now. | ||||||||||||||||||||||||||
| import RenderWorklet from "./render-worklet?worker&url"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Downloads audio from a track and emits it to an AudioContext. | ||||||||||||||||||||||||||
| // The user is responsible for hooking up audio to speakers, an analyzer, etc. | ||||||||||||||||||||||||||
| export class Audio { | ||||||||||||||||||||||||||
|
|
@@ -94,7 +91,11 @@ export class Audio { | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| effect.spawn(async () => { | ||||||||||||||||||||||||||
| // 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)), | ||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
Comment on lines
93
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Create the worklet node | ||||||||||||||||||||||||||
| const worklet = new AudioWorkletNode(context, "render"); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,9 +9,10 @@ | |
| "concurrently": "^9.1.2", | ||
| "cpy-cli": "^5.0.0", | ||
| "knip": "^5.60.2", | ||
| "prettier": "^3.6.2", | ||
| "rimraf": "^6.0.1", | ||
| "typescript": "^5.8.3", | ||
| "tsx": "^4.20.1" | ||
| "tsx": "^4.20.1", | ||
| "typescript": "^5.8.3" | ||
| }, | ||
| "packageManager": "[email protected]" | ||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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:
📝 Committable suggestion
🤖 Prompt for AI Agents