Unexpected import behaviour in "non-ssr" routes

[SourceR85]

Describe the bug

The Bug (or maybe a documentation issue?):

I've exported ssr=false in my root +layout. If I import (via statement/top-level) in any sub-route's +page.js/.ts a dependency that uses at the top level a browser API (more precise: any none-node API), the build failing "evaluating SSR" with an error:

ReferenceError [Error]: (the unknown API) is not defined

I expected, that SSR should be turned off for the page.js/.ts in that case :thinking:

Reproduction

Here you can find a prepared example for this miss-behaviour: https://github.com/SourceR85/pwa-load

At the end, it's pretty easy to replicate:

1. create a project with npm create svelte@latest xxx
2. add export const ssr=false; to a +layout.js/.ts
3. write some code, that depends (at the module scope) on non-node API's
4. Import that code in a "SPA" +page.js/.ts route, somewhere within the +layout path

Logs

❯ npm run build

> [email protected] build
> vite build


vite v4.5.1 building SSR bundle for production...
✓ 75 modules transformed.

node:internal/event_target:1083
  process.nextTick(() => { throw err; });
                           ^
ReferenceError [Error]: localStorage is not defined
    at file:///home/sourcer/dev/playground/pwa-load/.svelte-kit/output/server/entries/pages/_page.ts.js:4:24
    at ModuleJob.run (node:internal/modules/esm/module_job:217:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
    at async analyse (file:///home/sourcer/dev/playground/pwa-load/node_modules/@sveltejs/kit/src/core/postbuild/analyse.js:60:16)
    at async MessagePort.<anonymous> (file:///home/sourcer/dev/playground/pwa-load/node_modules/@sveltejs/kit/src/utils/fork.js:22:16)
Emitted 'error' event on Worker instance at:
    at [kOnErrorMessage] (node:internal/worker:326:10)
    at [kOnMessage] (node:internal/worker:337:37)
    at MessagePort.<anonymous> (node:internal/worker:232:57)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:807:20)
    at exports.emitMessage (node:internal/per_context/messageport:23:28)

Node.js v20.9.0

System Info

System:
  OS: Linux 6.6 Fedora Linux 39 (KDE Plasma)
  CPU: (16) x64 AMD Ryzen 7 6800HS Creator Edition
  Memory: 3.05 GB / 13.34 GB
  Container: Yes
  Shell: 5.9 - /usr/bin/zsh
Binaries:
  Node: 20.9.0 - ~/.fnm/node-versions/v20.9.0/installation/bin/node
  Yarn: 1.22.21 - ~/.fnm/node-versions/v20.9.0/installation/bin/yarn
  npm: 10.1.0 - ~/.fnm/node-versions/v20.9.0/installation/bin/npm
  pnpm: 8.12.1 - ~/.fnm/node-versions/v20.9.0/installation/bin/pnpm
  bun: 1.0.1 - ~/.bun/bin/bun
npmPackages:
  @sveltejs/adapter-auto: 2.1.1 => 2.1.1 
  @sveltejs/kit: 1.30.3 => 1.30.3 
  svelte: 4.2.8 => 4.2.8 
  vite: 4.5.1 => 4.5.1

Severity

serious, but I can work around it

Additional Information

No response

[Rich-Harris]

This is expected. Pages can override layout options, which means we can't know whether the page should be server-rendered or not without loading the +page.ts. Put the $lib/webStorage import in your +page.svelte (which will not be imported on the server, if ssr = false) instead

[SourceR85]

Put the $lib/webStorage import in your +page.svelte (which will not be imported on the server, if ssr = false) instead

Thank you for your advise, but this store was only meant as a very abstract demo of the problem I stepped over.

In my production project, I import PouchDB to get some synchronized data in offline-cases, which then controls the behaviour of my load function (PouchDB depends on localStorage and IndexedDB). Setting up a similar project to demonstrate this behaviour, seems a little bit to much for me.

As a work around: using dynamic import() for the dependency works for me (line 15 in +page.ts). Maybe, this "bug" should be degraded to "annoyance" with high complexity?

On the other hand: the documentation didn't tell you, that ssr is required for +page.js/ts's (in that case, it's a documentation bug)

[Conduitry]

I'm labeling this as a documentation issue. There's not really a way for this to work without trying to evaluate +page.js files on the server, which necessarily also tried to evaluate any import ... from '...'s in them. We could maybe make it clearer which files will get evaluated on the server during build and thus need to not use any browser APIs in their top-level code (not import any code that uses browser APIs at the top level).

[SourceR85]

As far as i know by now: The only information, that decides if a +page.js/ts needs to be ssr'd is an export of ssr, right?

If so, here my thoughts: After evaluating a +layout, with that ssr=false hint, I can slam a regex somewhere to extract that hint from child pages and layouts. Will it be enough to avoid running/executing that file(s) in build or need svelte/kit more data, I'm currently unaware of?

[Rich-Harris]

That's called static analysis and it's one of those things that seems like a good idea until you try it. Let's say you try the regex approach — what do you do here?

// export const ssr = false;

Next you could try parsing the code and analysing the AST, but then what do you do here?

export const ssr = EXPERIMENTS_USE_SSR;

Maybe you could special-case environment variables like that, but what happens when you have something like this?

export { ssr, prerender } from '$lib/config';

And so on. It's tempting to say 'we could try static analysis first and then fall back to runtime analysis', but then the system becomes very hard to reason about — a seemingly unrelated change could cause the sort of failures you encountered, and it would be far from obvious how to solve them. It's much better if the rules are simple and well-understood, and in this case there are two solutions (in order of preference, but also decreasing order of ease):

• send a PR to PouchDB so that it handles the 'oh, look, I'm running in Node' case gracefully
• the dynamic import(...) approach you already landed on

[AKuederle]

I ran into a similar issue (and looking through the issues a couple of people did). I understand the rational why this can not be easily circumvented, but both "workarounds" (modify the library used, use dynamic imports everywhere), seem unsatisfying.

From my limited understanding of the internals and rational behind the design, it feels like the issue stems from having +page.ts and +layout.ts as files that mix both logic and config, which is create, if the config depends on the logic (e.g. dynamically switching ssr), but not great if the logic depends on the config (e.g. when you want to use browser only libraries with import side-effects.

A "simple" (based on my naive understanding) workaround would be to introduce a additional optional file (e.g. +config.ts) that is read first. As this config file is not the place where you are expected to put any core logic, it is easier to get rid of any problematic imports there.

An issue arises when you want to support cases, where configuration of a child route placed in +layout/+page.ts should overwrite config from a +config.ts in a parent route. As it might not be clear what to do then, but there might a couple of ways to work around that too.

The other (less granular) option, feels like to really have a global "SPA" switch. I guess most people running into this issue (myself included), were trying to build "just a simple SPA", but were surprised when suddenly code ran in a NodeJS env during dev.

[AKuederle]

Just as an additional note: It might be possible to pull that off even with the existing structure using the *.server.ts files. Basically, the only change that would be required is that if we find a ssr=false in a page.server or layout.server file, we will not load the page.ts or layout.ts file. Would that make sense in the current resolution order?

[ccmobias]

I recently encountered this problem in my project as well. Before 1.0.0-next.405, I set the global ssr to false directly in the handle function of hooks.js and it works well. But after I update sveltekit to 1.0.0-next.405, this problem arose because the render_page function of /src/runtime/server/page/index.js will try to get nodes and load +page.js files.

I understand this problem may not be easy to circumvent and I think the dynamic imports should work for me. But it's a bit strange to me that I already set global ssr to false to build "just a simple SPA" (according to AKuederle) but browser APIs is still run when build the project.