breaking: delete $service-worker module#16450
Open
Rich-Harris wants to merge 15 commits into
Open
Conversation
|
Install the latest version of pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/f4b122b1e2df258257db2a27e8decb6cace809f7Open in |
🦋 Changeset detectedLatest commit: f4b122b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
|
Have you looked into using multiple tsconfig files for this? I wonder if it's possible to do this nicely, since sveltekit is already using a generated tsconfig. Could even go further and remove browser types from |
Member
|
Not sure if language tools supports multiple tsconfigs already but that would be nice. Vite vanilla template already does this too |
Member
Author
Yeah, shortly after writing that I reached the conclusion that including the service worker in the default project was a non-starter. For it to just work OOTB I think we need to have a |
9 tasks
Rich-Harris
added a commit
that referenced
this pull request
Jul 24, 2026
Pairs with #16450. The way we handle TypeScript configuration is a little bit messy, particularly as it regards service workers, which need to be in their own TypeScript project. This PR tidies things up a bit. It will need corresponding work to happen to the default project template. Essentially, a SvelteKit 3 project only needs this `tsconfig.json` file: ```json { "extends": "$app/tsconfig" } ``` A service worker should live in a `src/service-worker/index.ts` file (_not_ a `src/service-worker.ts` file, though this will still work) with a sibling `tsconfig.json` like this: ```json { "extends": "$app/tsconfig/service-worker" } ``` You can add whatever other configuration you like there, though it's important that the first one `"excludes"` the service worker, and that both contain `"types": ["$app/types"]` if those options are overridden. In addition to those two modules — which are 'real' modules in the sense that they live in `node_modules/$app` — we introduce a third, `$app/service-worker`, which exists solely to make it easier to get a correctly-typed `self` object (without this, there is a lot of [annoying boilerplate](https://svelte.dev/docs/kit/service-workers#Inside-the-service-worker)): ```ts import { self } from '$app/service-worker'; ``` > In future, we could also use this module for e.g. offline caching strategies and the like. I've tested the `node_modules/$app` approach with pnpm and it seems like a winner. `$` is an invalid character for an npm package but valid for Node's module resolution algorithm, so it works without danger of clobbering. pnpm doesn't nuke the directory when it installs or uninstalls packages (much like things like `node_modules/.vite`). We could extend this idea to other modules, which would help with [this very longstanding issue](#1485). TODO: - [x] docs - [x] validation of `types` and `exclude` - [x] tidy up --- ### Please don't delete this checklist! Before submitting the PR, please make sure you do the following: - [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] This message body should clearly illustrate what problems it solves. - [x] Ideally, include a test that fails without this PR but passes with it. ### Tests - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check` ### Changesets - [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running `pnpm changeset` and following the prompts. Changesets that add features should be `minor` and those that fix bugs should be `patch`. Please prefix changeset messages with `feat:`, `fix:`, or `chore:`. ### Edits - [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed. --------- Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
…ing the functionality back to `$app/manifest`, we can
Rich-Harris
marked this pull request as ready for review
July 24, 2026 16:44
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
…es(url.pathname)` can never match because `$app/manifest` paths are base-relative (no leading slash) while `url.pathname` is absolute, so the cache-first branch is dead code.
This commit fixes the issue reported at documentation/docs/30-advanced/40-service-workers.md:69
## Bug
The service worker example in `documentation/docs/30-advanced/40-service-workers.md` builds its asset list from `$app/manifest`:
```js
const ASSETS = [
...immutable.map((asset) => asset.path), // the Vite output
...assets.map((asset) => asset.path) // everything in `static`
];
```
Per `packages/kit/src/types/ambient.d.ts` (lines 65–74), the `path` values from `immutable`/`assets` are **relative to the base path with no leading slash** (e.g. `_app/immutable/...`). This is confirmed in `packages/kit/src/exports/vite/index.js`, where `immutable` is generated as `{ path: file }` from Vite manifest keys and `assets` as `{ path: asset.file }` — both bare relative paths.
In the `fetch` handler:
```js
const url = new URL(event.request.url);
...
if (ASSETS.includes(url.pathname)) { // url.pathname is ALWAYS absolute, e.g. "/_app/immutable/..."
const response = await cache.match(url.pathname);
...
}
```
`url.pathname` is always absolute (leading `/`, plus any base prefix). Since `ASSETS` contains only bare relative strings, the equality-based `Array.includes()` check **never matches**. The entire cache-first branch — the primary thing the example is meant to demonstrate — is dead code.
This is a regression from the earlier `$service-worker`-based example, where `build`/`files` were absolute paths (`base + '/file'`) that did match `url.pathname`. The `install` step happens to still work only because `cache.addAll` resolves the relative URLs against the service worker's scope.
**Concrete trigger:** any GET request for a build/static asset (e.g. `/_app/immutable/entry/start.<hash>.js`) enters `respond()`; `ASSETS.includes('/_app/immutable/entry/start.<hash>.js')` returns `false` because `ASSETS` contains `_app/immutable/entry/start.<hash>.js` (no leading slash), so it falls through to the network-first path instead of serving from cache.
## Fix
Resolve the base-relative manifest paths to absolute pathnames using `resolve()` from `$app/paths` (which is available in service workers — see `packages/kit/test/apps/options-2/src/service-worker.js`, which imports and uses it):
```js
import { resolve } from '$app/paths';
...
const ASSETS = [
...immutable.map((asset) => resolve(asset.path)),
...assets.map((asset) => resolve(asset.path))
];
```
For an input that does not start with `/`, `resolve` returns `base + '/' + path` (see `packages/kit/src/runtime/app/paths/client.js`), producing an absolute pathname such as `/_app/immutable/...` (or `/base/_app/...` with a configured base) that matches `url.pathname`. `cache.addAll(ASSETS)` continues to work with these absolute pathnames.
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: Rich-Harris <hello@rich-harris.dev>
…udes"`/`"excludes"` instead of the correct `"include"`/`"exclude"`, so copying it fails to exclude the service worker from root type-checking
This commit fixes the issue reported at documentation/docs/30-advanced/40-service-workers.md:114
## Bug
In `documentation/docs/30-advanced/40-service-workers.md` the "Type safety" section shows a root `tsconfig.json` example:
```json
{
"extends": "$app/tsconfig",
"includes": ["src", "test"],
"excludes": ["src/service-worker"]
}
```
TypeScript's config schema uses the **singular** keys `include` and `exclude`. The plural forms `includes`/`excludes` are unknown top-level keys, which TypeScript silently ignores. A user copying this example verbatim would therefore **not** exclude `src/service-worker` from the root project, so the service worker code would still be type-checked by the root project with the wrong `lib`/types — defeating the entire purpose of the section.
Confirmed against the actual generated config in `packages/kit/src/core/sync/write_tsconfig/index.js` (emits `exclude` and `include`) and `packages/kit/test/apps/basics/tsconfig.json` which uses `"include"`.
## Fix
Changed `"includes"` → `"include"` and `"excludes"` → `"exclude"` in the example.
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: Rich-Harris <hello@rich-harris.dev>
…ltejs/kit into delete-service-worker-module
elliott-with-the-longest-name-on-github
approved these changes
Jul 24, 2026
elliott-with-the-longest-name-on-github
left a comment
elliott-with-the-longest-name-on-github
left a comment
Contributor
There was a problem hiding this comment.
lgtm, just some questions / nits
…set without `posixify()`, so on Windows it never matches the posix-normalized importer keys and transitive server-only imports into a service worker go undetected. This commit fixes the issue reported at packages/kit/src/exports/vite/index.js:792 ## Bug In `plugin_guard`'s `load` handler (`packages/kit/src/exports/vite/index.js`), the import graph is walked to detect server-only code leaking into client entrypoints. The importer keys stored in `import_map` are produced by `normalize_id(importer, ...)` (line 744), and `normalize_id` ends with `return posixify(id)` — so **every key uses forward slashes**. The `entrypoints` set is compared against those keys via `entrypoints.has(importer)` (line 817). All entrypoints derived from `manifest_data` are already posixified. However, the service worker entrypoint was added as: ```js entrypoints.add(path.relative(root, service_worker_entry_file)); ``` `path.relative` returns platform separators, so on Windows this yields e.g. `src\service-worker.js`, which can never equal the posixified importer key `src/service-worker.js`. ## Trigger On Windows, a project with a `src/service-worker.js` that transitively imports a `$env/dynamic/private` (or otherwise server-only) module. The guard's `find_chain` walk reaches the service worker importer but `entrypoints.has(importer)` returns `false`, so the server-only-import error is silently not raised — weakening a correctness/security guard. (Note: even though `service_worker_entry_file` is posixified elsewhere at line 1059, `path.relative` re-introduces backslashes on Windows regardless of input.) ## Fix Wrap with `posixify()` to match the pattern used for all other entrypoints: ```js entrypoints.add(posixify(path.relative(root, service_worker_entry_file))); ``` `posixify` is already imported at the top of the file (line 15). Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com> Co-authored-by: Rich-Harris <hello@rich-harris.dev>
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
4 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.
This deletes the weirdo
$service-workermodule now that it's no longer necessary (because of$app/manifestand the availability of$app/envand$app/pathsin the service worker) and overhauls our docs to use the new$app/tsconfig/service-workeretcPlease don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits