Skip to content
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

Avoid loading +page.js/+layout.js files on server in SPA mode #12580

Open
janopae opened this issue Aug 15, 2024 · 22 comments
Open

Avoid loading +page.js/+layout.js files on server in SPA mode #12580

janopae opened this issue Aug 15, 2024 · 22 comments
Labels
feature request New feature or request
Milestone

Comments

@janopae
Copy link

janopae commented Aug 15, 2024

Describe the bug

Even though I did everything the docs provide in order to disable any form of server side rendering or static site generation, the dev server tries to execute my code on the server.

Reproduction

npm create svelte@latest my-app
cd my-app
npm install

Edit src/routes/+layout.ts according to https://kit.svelte.dev/docs/single-page-apps

export const ssr = false;
export const prerender = false;

Edit svelte.config.js according to https://kit.svelte.dev/docs/single-page-apps#usage

import adapter from '@sveltejs/adapter-static';
import { vitePreprocess } from '@sveltejs/vite-plugin-svelte';

/** @type {import('@sveltejs/kit').Config} */
const config = {
	// Consult https://kit.svelte.dev/docs/integrations#preprocessors
	// for more information about preprocessors
	preprocess: vitePreprocess(),

	kit: {
		// See https://kit.svelte.dev/docs/adapters for more information about adapters.
		adapter: adapter({
			fallback: 'index.html'
		}),
	}
};

Create some code that will only work on client side

// +layout.ts

localStorage.setItem('test', 'lol');

Start the dev server

npm run dev

Open the website in browser, and there will be an error complaining about localStorage not being definded.

Logs

No response

System Info

System:
    OS: Linux 5.15 Ubuntu 22.04.4 LTS 22.04.4 LTS (Jammy Jellyfish)
    CPU: (4) x64 Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz
    Memory: 870.74 MB / 1.91 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 20.5.1 - /usr/bin/node
    Yarn: 1.22.22 - /usr/bin/yarn
    npm: 9.8.0 - /usr/bin/npm
  npmPackages:
    @sveltejs/adapter-auto: ^3.0.0 => 3.2.2 
    @sveltejs/adapter-static: ^3.0.2 => 3.0.2 
    @sveltejs/kit: ^2.0.0 => 2.5.20 
    @sveltejs/vite-plugin-svelte: ^3.0.0 => 3.1.1 
    svelte: ^4.2.7 => 4.2.18 
    vite: ^5.0.3 => 5.3.5

Severity

serious, but I can work around it

Additional Information

No response

@janopae
Copy link
Author

janopae commented Aug 15, 2024

Update: It seems like this also stops the build process, not just the dev server.

Is there really no way to only target a browser if that's the only place your code is going to run in?

@dominikg
Copy link
Member

please provide a link to a repository with a complete and minimal reproduction. You mention a custom adapter and the filename you use layout.ts is missing the + at the start.

@janopae
Copy link
Author

janopae commented Aug 16, 2024

My apologies – the custom adapter and the missing + were both a mistake. I use a custom adapter in the app I develop, but I could reproduce this with adapter-static.

I created a repository to reproduce the problem: https://github.com/janopae/reproduce-sveltekit-tries-to-run-ssr-code-even-with-ssr-disabled

@janopae
Copy link
Author

janopae commented Aug 16, 2024

It seems like SvelteKit has to execute the +layout.ts, otherwise it wouldn't even know that SSR is disabled.

So we need either another way to add layout code that won't be used at compile time to determine wether SSR is enabled (would be a weird twist on the naming of the file), or we need a way to disable SSR from the begin with (e. g. in the svelte.config.js).

@dominikg
Copy link
Member

dominikg commented Aug 16, 2024

you can't use localStorage in +layout.ts without an if(browser) guard, spa mode or not.

also you have to put code in exported functions, not top level.

@david-plugge
Copy link
Contributor

david-plugge commented Aug 17, 2024

Im pretty sure your code works IF you put it inside a load function. The file has to be evaluated so sk can read the ssr setting but if SSR is off, the load function is not executed on the server.

@janopae
Copy link
Author

janopae commented Aug 18, 2024

Alright. I think this is an unexpected gotcha that could be avoided with a general ssr switch in the svelte.config.js, which would turn this issue into a feature request. But if there are reasons I don't know about why we can't have a switch in the top level configuration and you think it should stay this way, feel free to close this issue.

Personally, I reconsidered the use of SvelteKit for my use case, as there have been too many gotchas that took me a lot of time to resolve, and they keep coming, mostly relating to disabling SSR:

  • places in which browser APIs can't safely be used (e. g. top level of +layout.{ts|js}) – this might create some "time bombs", when library code (both npm and self-written one in the lib directory) uses browser API behind the scenes, maybe just under certain conditions (this is basically the problem imposed by this issue, but also that one: Unexpected import behaviour in "non-ssr" routes #11310)
  • server code always gets generated and gets thrown away if not needed (Disable generation of server code #12555)
  • "getting started" guide and other docs often assume you execute code on server, a lot of them don't work without ssr which is not always obvious

Especially, developing a browser extension using sk seems impossible at the moment, because both prerendered and "fallback" routing code can only be executed with the CSP setting "unsafe-inline", which can't be enabled for browser extensions (#11009).

I'd love to see the SvelteKit homege communicating its focus on deployments with a node server, as currently, the website presents SvelteKit as "super flexible" and especially mentions SPAs without node backends as a use case.

@bholmesdev
Copy link

bholmesdev commented Sep 7, 2024

Thanks for the thorough overview @janopae. I'm early on my SPA journey with SvelteKit, and you point out some gotchas I have yet to hit.

I personally didn't reach for SvelteKit for the "typical" reasons. I wanted to use Svelte to create a client application, but there simply aren't viable CSR routers for Svelte other than SvelteKit. My use case is building a local-first application that relies on SQLite in the browser, and I use +page.ts files to execute queries against this database before displaying the page. Obviously, this cannot run in a server context. I managed to get this working client-side with a browser guard in front of my database object, but I do worry how brittle this could get overtime.

Personally, I expected a +page.client.ts file that mirrors the format of +page.server.ts. It seems odd that you can create a server-side loader and a "universal" loader but not a client loader. This reflects the vibe I got trying to build an SPA: the authors do not believe client SPAs are a good practice, so the experience is second-rate. I'm hoping new architectures like local-first prove where client-side applications are a good idea.

@ghostdevv
Copy link
Member

Personally, I expected a +page.client.ts file that mirrors the format of +page.server.ts. It seems odd that you can create a server-side loader and a "universal" loader but not a client loader.

I think it makes sense to not have the +page.client.ts - if you're using SSR, only running on the client could cause hydration issues, and if you're CSR, it doesn't matter. However, I do agree that SvelteKit importing / evaluating +page.ts files on the "server" when SSR is disabled is annoying.

I've felt the pain of that myself plenty of times when building client-side apps, and I understand the frustration when considering past comments denouncing SPAs. However, we want SvelteKit to be the best tool for the job, even for SPAs!

there simply aren't viable CSR routers for Svelte other than SvelteKit

There are other options such as Routify, and numerous other declarative ones available. Routify v2 specifically should work just fine if you're using Svelte 3/4

@braebo
Copy link
Member

braebo commented Sep 7, 2024

Funny, @Rich-Harris spoke on this exact in a recent interview!

But, you know, amongst that, I at least am definitely thinking about what are the ideal integration points between the rendering framework and the data if the data exists in this sort of local first context.

@bholmesdev
Copy link

bholmesdev commented Sep 8, 2024

@ghostdevv Appreciate the thorough response here!

I do agree that SvelteKit importing / evaluating +page.ts files on the "server" when SSR is disabled is annoying.

Yeah that's all I'm getting at. I totally agree that +page.client.ts isn't a nice API. My understanding is that +page.ts must be executed at build-time to read the export const ssr flag, specifically for dynamic values like export const ssr = import.meta.env.DEV. My suggestion of +page.client.ts was to find some way to avoid server-side evaluation of these files entirely.

Also have a suggestion from maintaining Astro: it may help to remove dynamic value support for export const ssr so exports can be traced with an export crawler, without executing the file to determine the value. This was our approach for our prerender flag.

And I appreciate those links to community routers! I just prefer to use solutions that are first-party maintained and have a diverse contributor graph. I hadn't noticed the latest Routify v3 release though. May be worth a second look! In a perfect world, I'd use a SPA router within Astro ;)

@benmccann benmccann added feature request New feature or request and removed awaiting submitter labels Sep 9, 2024
@benmccann benmccann added this to the 3.0 milestone Sep 9, 2024
@benmccann benmccann changed the title Dev server tries to execute code on server, even though ssr=false and adapter static Avoid loading +page.js files on server in SPA mode Sep 9, 2024
@benmccann benmccann changed the title Avoid loading +page.js files on server in SPA mode Avoid loading +page.js/+layout.js files on server in SPA mode Sep 9, 2024
@benmccann
Copy link
Member

benmccann commented Sep 9, 2024

Thanks for pointing this out guys. I think it's an important issue, so added it to the SvelteKit 3 milestone to make sure we take a look at what can be done better here. I've heard at least a few options:

  • stop supporting runtime config for that option
  • do a best-effort static analysis for that option before checking runtime config. Most SPA users will set ssr = false in their layout file and that's it. We could during build add values that can be determined statically or are undeterminable to the ssr manifest
  • add a client-only setup API (would only solve access of browser-only APIs during setup and would complicate our API, so I prefer one of the above solutions)

This is important as many people build SPAs in order to support things like capacitor and tauri and want to be able to directly reach for browser-only APIs without having to add if (browser) checks since they know their app will never have to run on a server

@flpms

This comment was marked as off-topic.

@paoloricciuti

This comment was marked as off-topic.

@flpms

This comment was marked as off-topic.

@paoloricciuti

This comment was marked as off-topic.

@benmccann

This comment was marked as off-topic.

@Rich-Harris
Copy link
Member

This issue exists in another form. When you run vite dev, your code is running in Node. But your app might use a different runtime in production, for example Cloudflare Workers or Vercel Edge Functions or Deno or Bun or what-have-you.

This means that the code you write has to satisfy two constraints:

  • it must run in the version of Node Vite is running in
  • it must run in $TARGET_ENVIRONMENT

If you squint, this is fine — JS runtimes are constantly converging, and indeed there's a cross-platform effort to define an interoperable platform. But it means that you can't, I don't know, import from bun:sqlite or otherwise use platform-specific APIs.

This is, of course, the same problem with client-side only SPAs — the code in your +layout.js and +page.js must satisfy those same two constraints, where $TARGET_ENVIRONMENT is the browser. So no window, no document.

Nor is this a dev-only problem. At build time, we evaluate all the files that define route config (albeit in a child Node process) so that we know which routes need to be prerendered, and so that we can pass route config to adapters (so that e.g. adapter-vercel can generate some non-prerendered routes as edge functions, and others as regular lambda functions).

With the forthcoming release of the Vite Environment API, we're investigating ways to do SSR in non-Node environments. For example, #12637 has a proof-of-concept implementation of server-side rendering in workerd. But this doesn't fully solve the issue, because different routes might have different requirements — for example some routes might be prerendered and need to use node:fs, and an app deployed to Vercel can mix and match edge and non-edge runtimes.

So there's a common requirement here: we need to be able to express which server runtime applies to a given route without running code. I don't like the 'stop supporting runtime config' or 'do a best-effort static analysis' approaches. Runtime config is very useful (e.g. setting stuff based on environment variables) and necessary for things like entries which is arguably included in 'config'. 'Best-effort' is acceptable for optimisations, but not for anything load-bearing. I especially dislike static config in a place where it looks like you should be able to run code — apologies to e.g. Astro and Next which both do this — because it puts you in an uncanny valley.

If we accept the premise that you should be able to run code to generate your config, the only thing we need to be able to determine without running code is where to run the code. In other words the only thing that can't be runtime config is, well, the runtime config.

There's a few ways we could go about this. It could be a new file (+config.json or whatever), but I think the nicer approach would be to have some kind of directive at the top of the route file — maybe a pragma ("use workerd", "use node@22", etc), maybe a comment (// @sk-runtime=edge or whatever). The usual precedence rules would apply (i.e. a +page could override a +layout), and the string could be passed to the adapter to select one of potentially multiple environments, falling back to a default (if no runtime was specified) or a built-in one (if e.g. node was specified for a prerendered route in an app using a non-Node adapter). To prevent stuff running outside the browser at all you'd specify 'none' (which would be disallowed for a route that had +layout.server.js or +page.server.js files).

Note that this all overlaps with export const ssr but is also independent of it — you could have a route that is only rendered on the client, but still has a load function that you want to run on the server (avoiding the roundtrip on startup to get data). Today, if ssr is false we don't run load on the server — we suffer the roundtrip — but that seems like something we should fix.

Anyone have thoughts on what the nicest version of this looks like?

@Rich-Harris
Copy link
Member

Currently leaning towards the pragma ("use my-runtime"):

  • it's familiar, both to those of us sufficiently long in the tooth to remember "use strict" and to people using modern React with "use server" and "use client"
  • by extension, people are unlikely to think you can do `use ${runtime}`;
  • it's the same category of thing as "use strict" etc — it affects how something gets evaluated before it gets evaluated
  • Tooling like ESLint already knows about pragmas (it will complain about "use blah" in the middle of your module, but not if it's at the top)
  • while there is some precedent for using comments instead (e.g. // @ts-check) these don't generally affect how the code actually runs
  • also, comments are by their nature less visible

For disabling server-side evaluation altogether, "use browser" seems best. It's more explicit than "use none" (even if arguably less 'correct', given that it's selecting a server runtime), and less likely to lead to confusion when googling than "use client".

@benmccann
Copy link
Member

What happens to "use my-runtime" during build? Will it be stripped out? Or will it be sent to the browser and the browser will ignore it if it's something it doesn't recognize?

@Rich-Harris
Copy link
Member

Minifiers will strip it out

@janopae
Copy link
Author

janopae commented Sep 24, 2024

As the same pragma syntax is used for "strict", maybe indicating being about runtimes would increase readabilty and googlability? Something like "use runtime browser";, "use run-in-browser"; or "use dont-run-on-server";.

(This might just be bikeshedding)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

10 participants