[feat] SSR never option

[JeanJPNM]

This pull request aims to close #1650 by adding a never value to the ssr option, preventing svelte kit from importing the files on the server altogether.

Other alternatives: add spa option to config.

Todo list:

• Build simple approach
• Add tests
• Document feature
• Add changeset

[changeset-bot]

🦋 Changeset detected

Latest commit: be01755

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

[JeanJPNM]

Hey @benmccann, do you how I can create a new test app?

[JeanJPNM]

Do you think the docs are clear enough now?

[abalmos]

Is there any known workaround while this PR bakes? As far as I can tell, this issue completely blocks final building our SPA that uses web workers extensively.

[JeanJPNM]

Here are the current workarounds

[Rich-Harris]

Thanks for the PR. I'm -1 on this — this has been discussed in the maintainer's chatroom but not here, I think, so let me explain:

In general, we want to encourage the use of SSR as widely as possible. We therefore don't want people to solve the 'I can't SSR this particular page' problem by disabling SSR for the entire app. While there are situations where disabling SSR app-wide is the correct thing to do, in the many cases where that's not the case, the existence of this option would encourage bad practices.

I think something like the following is preferable:

export function handle({ request, resolve }) {
  return resolve(request, { ssr: false });
}

That way it can easily be scoped to specific parts of the app, for example, or even conditionally enabled for some UAs. It requires some discussion (not sure if adding options to resolve makes the sequence helper too complicated, for example) but I think that's the right direction to go in.

[Conduitry]

I might suggest resolve({ request, ssr: false }) while we can still sneak in breaking changes. We had opted for positional arguments at the time because there was only one and we only foresaw there ever being one, but once we have two, I think it makes sense to make them all named, not just the new options.

[benmccann]

Rich's suggestion would allow you to completely disable SSR if you'd like. It'd be more powerful though - it would let you decide whether to disable on a per-page / per-request basis. If you really have a need to always set it to false you could do that still, but it would also give you far more power to do things like enable it only for GoogleBot, disable only for /admin route, or whatever else you'd like to do. I think making it more flexible in that manner is a good idea

[JeanJPNM]

I think something like the following is preferable:

export function handle({ request, resolve }) {
  return resolve(request, { ssr: false });
}

That way it can easily be scoped to specific parts of the app, for example, or even conditionally enabled for some UAs.

That's a great solution, but it doesn't solve one problem: the final app bundle. All sveltekit pages and their dependencies are bundled into app.js regardless of whether they actually need to be. Moving ssr evaluation to handle makes it too flexible, because we can't make it clear which pages will be SSRd at compile time, so every page will get bundled and that would result on the same x is not defined errors for side effect imports.

[benmccann]

@JeanJPNM that's a great point.

That's because Kit sets inlineDynamicImports to true by default:

kit/packages/kit/src/core/build/index.js

Line 445 in d15e369

inlineDynamicImports: true

I'm not really sure that we need to though, so perhaps we can change it

[benmccann]

is there any harm in having metadata_lookup populated even if it's not used? I wonder if it might be simpler to just leave the code as is and not add an extra condition, but want to make sure I'm not missing anything

[JeanJPNM]

Not really, I just thought that it would be better to not populate metadata_lookup when it isn't going to be used.

[benmccann]

@JeanJPNM I tested and the only thing we need is indeed to set inlineDynamicImports: false in the config. I sent a PR to do that by default: #2753. In the meantime, you should be able to set it manually in the svelte.config.js to unblock development

[JeanJPNM]

With that problem out of the way, we can implement @Rich-Harris's solution without any drawbacks (though I think that option should be renamed to something like loadPage)

[benmccann]

I much prefer ssr to loadPage as a name

[JeanJPNM]

Closing in favor of #2804.