Repository navigation
fix(router): bundle service workers into a single classic script - #9227
Conversation
Service workers are registered as classic scripts, but the client build emitted them as ES modules importing shared chunks, so registering one that imported any module failed. The import paths were also relative to the chunk's original location in build/, which the rename to /service-worker.js broke. Inline the imported chunks into an IIFE before the rename. Fixes QwikDev#8075
🦋 Changeset detectedLatest commit: 93b34c6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The nested build wrapped inlined dynamic imports in Vite's __vitePreload again. Its error path uses window, which service workers lack, so a failing import threw a ReferenceError instead of its own error.
The bundle is already keyed by file name, and the e2e test now covers bundling before the rename.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Bundle service-worker chunks packages/qwik-router/src/buildtime/vite/bundle-service-worker.ts, packages/qwik-router/src/buildtime/vite/bundle-service-worker.unit.ts |
The new bundling function rebuilds chunks with static or dynamic imports as non-splitting IIFE scripts, then clears their import lists. Unit tests cover static and dynamic imports, and chunks without imports. |
Integrate bundling with Vite output packages/qwik-router/src/buildtime/vite/plugin.ts |
The asynchronous generateBundle hook recovers route-loader hashes and replaces placeholders. For client builds, it bundles service-worker chunks before assigning configured filenames. Entry chunks are matched by normalized, extensionless facade paths. |
Exercise shared-module service workers e2e/adapters-e2e/src/greeting.ts, e2e/adapters-e2e/src/routes/greeting/index.tsx, e2e/adapters-e2e/src/routes/service-worker.ts, e2e/adapters-e2e/tests/service-worker.spec.ts, .changeset/service-worker-classic-script.md |
The end-to-end test checks the greeting page and the service-worker message response using shared code. A patch changeset records the fix. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix · Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
participant QwikRouterPlugin
participant bundleServiceWorkerChunks
participant OutputBundle
QwikRouterPlugin->>bundleServiceWorkerChunks: Pass service-worker chunks and output bundle
bundleServiceWorkerChunks->>OutputBundle: Resolve and load imported chunks
bundleServiceWorkerChunks-->>QwikRouterPlugin: Return bundled code and clear import lists
Suggested reviewers: wmertens
Merge Risk: 
93b34
-
No actionable merge-blocking issue is established by the supplied evidence; the change appears ready for normal merge checks.
Security architecture risk: 
377d7
-
The change repairs classic service-worker output without an identified increase in attacker-controlled input or privileges. Remaining uncertainty concerns unusual build-failure recovery and deployed worker update behavior, rather than a demonstrated security flaw.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — The production effect reaches applications whose client builds contain service workers with recorded imports. The changed path transforms developer-built executable code; the inspected call chain does not connect browser message data to build inputs or introduce tenant, credential or infrastructure authority.
Trust Boundaries and Controls
- observed — The custom loader serves code only for synthetic identifiers that map to bundle chunks. Unhandled resolutions return null to Vite, so this mechanism is not an import sandbox or a complete filesystem-isolation boundary.
Resilience and Maintainability Implications
- inferred — Sequential processing avoids concurrent worker mutations in the production caller. A worker is committed only after successful rebundling, and cleared import lists make subsequent processing skip it. Error propagation provides build-stage containment, but reuse of a partially transformed bundle after a caught failure is not demonstrated.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: bundling service workers into a single classic script. Linked Issues check Issue [ #8075] requires Qwik City to inline imported modules in generated service workers so classic-script registration does not fail onimportsyntax. The PR addsbundleServiceWorkerChunks, rebun…Out of Scope Changes check The changes stay within issue [ #8075]. The bundling helper andgenerateBundleintegration implement the service-worker fix. The sharedfindEntryChunkhelper supports the required lookup and rename…Full details: Linked Issues check
-
Explanation
Issue [
#8075] requires Qwik City to inline imported modules in generated service workers so classic-script registration does not fail onimportsyntax. The PR addsbundleServiceWorkerChunks, rebundles service-worker chunks as non-splitting IIFE output, and clears import metadata. The unit tests cover static imports, dynamic imports, classic-script execution, and unchanged workers without imports.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue [
#8075]. The bundling helper andgenerateBundleintegration implement the service-worker fix. The sharedfindEntryChunkhelper supports the required lookup and rename. The unit and adapter end-to-end tests verify the affected shared-module behavior. The changeset documents the router fix. No unrelated change is demonstrated.
-
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
This reverts commit b2012c3. It only changed the error a service worker reports when a module it loads with import() throws, and it relied on Vite's internal plugin names.
@qwik.dev/core
@qwik.dev/router
@qwik.dev/utils
eslint-plugin-qwik
create-qwik
@qwik.dev/optimizer
@qwik.dev/devtools
@qwik.dev/mcp
commit: |
github-project-automation
Bot
moved this from Waiting For Review
to Done
in Qwik Development
What is it?
Description
Fixes #8075.
Service workers are registered as classic scripts (
navigator.serviceWorker.register(url), notype: 'module'), but the client build emittedservice-worker.jsas an ES module chunk. As soon as the service worker imported anything, an npm package or a module shared with the app, the bundler split that code into its own chunk, and registration failed:A second problem: that import path is relative to the chunk's original location in
build/, and the chunk is renamed to/service-worker.jsafterwards, so the path would be wrong even with module service workers.This PR fixes both by inlining a service worker's imported chunks into a single IIFE in the client
generateBundle, before the rename. Service workers without imports are left untouched. The re-bundle runs on the already-built chunks with Vite'sbuild()(no new dependency) and uses the client build'sminifysetting.It also replaces the nested rename loop with a small
findEntryChunkhelper, so the service-worker lookup and the rename share it.Verification
bundle-service-worker.unit.ts: a static import and a dynamic import are each inlined, and the result runs as a classic script; a service worker without imports is unchanged. These tests failed before the implementation.pnpm vitest run packages/qwik-router: 809 tests pass.pnpm tsc.checkand eslint pass.src/routes/service-worker.tsimports an npm package and a module shared with a route, then registered the output in a real browser (Edge via Playwright):main:ServiceWorker script evaluation failedI didn't run the full e2e suites locally.
Checklist
pnpm changeSummary by CodeRabbit