fix: make illegal server-only import errors actually useful by Rich-Harris · Pull Request #14155 · sveltejs/kit

Rich-Harris · 2025-08-08T21:10:59Z

fixes #14153. The one part I'm slightly nervous about is manipulating the errors that occur during the secondary build to avoid double-logging, though it's probably okay?

This is a radical simplification from what we had before. It results in consistent errors between dev and prod (no more 'build the app to figure out the import chain'), more readable errors (better formatting, more concise message, no useless stack trace), co-locates code better, is more efficient (instead of walking the entire module graph from the universal entry points, it walks up from a server-only module if/when one is found), and is a lot less code.

One very minor trade-off: it's no longer possible to differentiate between static and dynamic imports. I don't think this matters.

~~Not require ready to merge yet because FUCKING WINDOWS~~

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

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
This message body should clearly illustrate what problems it solves.
Ideally, include a test that fails without this PR but passes with it.

Tests

Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

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

Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

changeset-bot · 2025-08-08T21:11:03Z

🦋 Changeset detected

Latest commit: 21650f4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

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

Rich-Harris · 2025-08-08T21:24:49Z

I actually think we can get the import chain in development too, by inspecting the dev server's module graph. Tinkering on that locally

teemingc

LGTM. Might be good to also have a test for a transitive server import as I noticed that most of the tests are just a direct import.

teemingc · 2025-08-12T05:41:19Z

+				normalized === '$env/static/private' ||
+				normalized === '$env/dynamic/private' ||
+				normalized === '$app/server' ||
+				normalized.startsWith('$lib/server') ||


I wonder if we can easily change this to only match $lib/server/ directories or $lib/server.* files to fix #14069 but I guess that can be its own PR after this one

ah yeah this should really just add a trailing slash. We don't state anywhere that $lib/server.ts should be considered a server-only module — just this:

You can make your own modules server-only in two ways:

adding .server to the filename, e.g. secrets.server.js

placing them in $lib/server, e.g. $lib/server/secrets.js

Unfortunately we do currently regard $lib/server.ts as server-only, so the simple fix would be a breaking change. I think we should fix that in SvelteKit 3 and warn on it in the meantime. Will do it as a separate PR

Actually I've changed my mind. If we think it's okay to fix #14069 in a patch release then the same is true of $lib/server.ts. Just gonna update .startsWith('$lib/server') to .startsWith('$lib/server/')

svelte-docs-bot · 2025-08-12T16:02:32Z

Preview: https://svelte-dev-git-preview-kit-14155-svelte.vercel.app/

…#14155) * hide useless stack * defer to graph_analysis in build, so the more useful error appears * tweak error message * various fixes * prevent double-logging * tidy up a lil * changeset * unify * tidy up * tweak * unused * simplify, update tests * simplify * simplify/fix * tweak * fix tests * try this * while we're here * getting closer * ahhh i think this is it * last bit * Update packages/kit/src/exports/vite/utils.js * Update packages/kit/test/apps/dev-only/test/test.js * test for indirect import

Addresses part of #15461 `vite-plugin-sveltekit-guard` builds its `import_map` in every environment, but the map is only read in the `load` hook, which bails for non-client consumers before touching it. On the svelte.dev build that makes 69% of the guard's `resolveId` calls (3,987 of 5,778) dead work, and the guard was the top entry in dominikg's `PLUGIN_TIMINGS` report in #13756. This adds the same consumer bail to `resolveId`. The error chains from #14155 are unaffected because the client environment collects its own edges. Measured on the svelte.dev build with `3.0.0-next.6` and vite `8.0.16` (details in #15461 (comment)), ssr-side hook time drops from ~296s cumulative to 23ms, the build succeeds, and a planted two-hop `$lib/server` violation still fails with the full chain. There is no new test because the behavior is meant to be unobservable and the exact chain assertions already exist in `test/apps/dev-only` (dev) and `test/build-errors/server-only.spec.js` (build), both green with this change. History for reviewers. #15439 added hook filters everywhere except this hook, which is blocked on vitejs/vite#21956. #15543 proposed this skip plus a node_modules importer skip and was closed pending kit 3. The ssr half was not fixed by kit 3. The node_modules half is not implemented here because it is unsafe, a library can legitimately import `$app/server` and its violation chain has to walk node_modules edges. --- ### 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.

Rich-Harris added 7 commits August 8, 2025 16:37

hide useless stack

26704c1

defer to graph_analysis in build, so the more useful error appears

6db5dcb

tweak error message

703d3e8

various fixes

4024d7c

prevent double-logging

49c126f

tidy up a lil

a0365e7

changeset

3aec108

Rich-Harris added 14 commits August 9, 2025 08:55

unify

966e021

tidy up

628c604

tweak

8fb34f6

unused

4b92500

simplify, update tests

f039ed5

simplify

95316ff

simplify/fix

fd093fb

tweak

0f0269d

fix tests

96264cc

try this

87ca7d0

while we're here

e2194e2

getting closer

157faa4

ahhh i think this is it

191a35f

last bit

49d5fbb

teemingc reviewed Aug 12, 2025

View reviewed changes

Comment thread packages/kit/src/exports/vite/utils.js

teemingc reviewed Aug 12, 2025

View reviewed changes

Comment thread packages/kit/test/apps/dev-only/test/test.js Outdated

teemingc approved these changes Aug 12, 2025

View reviewed changes

teemingc reviewed Aug 12, 2025

View reviewed changes

teemingc mentioned this pull request Aug 12, 2025

Better error when importing from $app/server in .remote file #14111

Closed

Rich-Harris commented Aug 12, 2025

View reviewed changes

Comment thread packages/kit/src/exports/vite/utils.js Outdated

Update packages/kit/src/exports/vite/utils.js

92a1825

Rich-Harris and others added 2 commits August 12, 2025 12:13

Update packages/kit/test/apps/dev-only/test/test.js

21650f4

test for indirect import

8cd0bba

Rich-Harris merged commit f4c9df6 into main Aug 12, 2025
21 checks passed

Rich-Harris deleted the gh-14153 branch August 12, 2025 20:14

github-actions Bot mentioned this pull request Aug 12, 2025

Version Packages #14187

Merged

This was referenced Sep 16, 2025

improve server-only and client-only error message vitejs/vite-plugin-react#863

Closed

fix(rsc): show import chain for server-only and client-only import error vitejs/vite-plugin-react#867

Merged

This was referenced Jul 17, 2026

Build performance investigation #15461

Closed

perf: skip import graph collection outside client environments #16383

Merged

Navigation Menu

Uh oh!

Uh oh!

fix: make illegal server-only import errors actually useful - #14155

fix: make illegal server-only import errors actually useful#14155
Rich-Harris merged 24 commits into
mainfrom
gh-14153

Rich-Harris commented Aug 8, 2025 •

edited

Loading

Uh oh!

changeset-bot Bot commented Aug 8, 2025 •

edited

Loading

Uh oh!

Rich-Harris commented Aug 8, 2025

Uh oh!

Uh oh!

Uh oh!

teemingc left a comment •

edited

Loading

Uh oh!

teemingc Aug 12, 2025 •

edited

Loading

Uh oh!

Rich-Harris Aug 12, 2025

Uh oh!

Rich-Harris Aug 12, 2025

Uh oh!

Uh oh!

svelte-docs-bot Bot commented Aug 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

Rich-Harris commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

Tests

Changesets

Edits

Uh oh!

changeset-bot Bot commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Rich-Harris commented Aug 8, 2025

Uh oh!

Uh oh!

Uh oh!

teemingc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teemingc Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rich-Harris Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Rich-Harris Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

svelte-docs-bot Bot commented Aug 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Rich-Harris commented Aug 8, 2025 •

edited

Loading

changeset-bot Bot commented Aug 8, 2025 •

edited

Loading

teemingc left a comment •

edited

Loading

teemingc Aug 12, 2025 •

edited

Loading