SvelteKit v1.0.0-next.304 is suddenly executing code during build, prerender problem?

[bluepuma77]

Describe the bug

I am building a full stack SvelteKit adapter-node application with database connection. It is build in a container to be run in a container. Todays npm update killed my build process because npm run build started to execute my source code.

So far this would build, but with SvelteKit v1.0.0-next.304 (and its dependencies) build started to fail:

// hooks.ts
export const handle = async ({ event, resolve }) => {
	// simulate crash because DB can not be connected 
        // without credentials and access during build process
	console.log('Going to simulate crash')
	process.exit();

	const response = await resolve(event);
	return response;
};

My last good npm update was 3 days ago.

Note: The build process always worked, but I had to use npm ci --production --ignore-scripts later with the built code.

This may have to do with the pre-rendering, but I have not touched those settings.

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-9crzvj?file=src%2Fhooks.js

const config = {
	kit: {
		prerender: {
			enabled: false
		}
	}
}

Run npm run build and it will execute the code and exit the process

Logs

No response

System Info

WORKS

  System:
    OS: macOS 12.2.1
    CPU: (8) arm64 Apple M1
    Memory: 105.05 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.19.0 - /opt/homebrew/opt/node@14/bin/node
    npm: 6.14.16 - /opt/homebrew/opt/node@14/bin/npm
  Browsers:
    Chrome: 100.0.4896.60
    Firefox: 98.0.2
    Safari: 15.3
  npmPackages:
    @sveltejs/adapter-auto: ^1.0.0-next.33 => 1.0.0-next.33
    @sveltejs/adapter-node: ^1.0.0-next.73 => 1.0.0-next.73
    @sveltejs/kit: ^1.0.0-next.303 => 1.0.0-next.303
    svelte: ^3.46.0 => 3.46.4

WORKS NOT ANYMORE

  npmPackages:
    @sveltejs/adapter-auto: ^1.0.0-next.34 => 1.0.0-next.34
    @sveltejs/adapter-node: ^1.0.0-next.73 => 1.0.0-next.73
    @sveltejs/kit: ^1.0.0-next.304 => 1.0.0-next.304
    svelte: ^3.46.6 => 3.46.6

Severity

blocking an upgrade

Additional Information

No response

[bluepuma77]

Setting full

prerender: {
	enabled: false,
	default: false,
	onError: 'continue',
},

does not help

[mrkishi]

Likely to have been introduced by #4443, since now we try creating a fallback regardless of prerender settings.

I'm not sure how we want to deal with it, though.

[dimfeld]

Why would you pre-render when prerendering.enabled = false? #4443 makes no sense to me.

Because we still need a fallback file when using the static adapter with prerendering disabled, otherwise you don't get a usable app at all. Perhaps the fallback file generation needs to be separated from the prerender process.

[bluepuma77]

@dimfeld So the solution would be to only ignore prerendering.enabled = false when using adapter-static.

Because it seems wrong for all the other adapters. We could completely remove prerendering.enabled.

[ponderingexistence]

By the way, I think the p0-urgent label is more appropriate for this.
It's breaking a lot of people's stuff.

[dimfeld]

If you read the second line of my comment I suggested that the fallback file generation be separated out. The way it works right now they’re very closely intertwined for reasons that I’m not sure of but probably just never mattered before the pretender process was moved into the build step.

Everyone lets all calm down please. It’s not like we’re out here trying to break things for you. I wasn’t freaking out on here when my build broke due to the earlier pre-rendering change. Instead I pinned my version to a working one and helped to figure out a solution. You would be good to do the same.

Just pin your version to 303 for now and we can all get this fixed in a calm manner.

[mrkishi]

Bugs eventually squeeze in unnoticed, and being on the @next edge of a fast-changing prerelease library means you'll eventually hit them. We welcome collaboration on getting things fixed, but pointing out how obvious a bug is from your perspective doesn't help anyone.

I'm bumping this to p0-urgent, but please let's strive to keep the discussion constructive—adding a thumbs up to the issue is enough to signal you were (unfortunately) affected by it.

[bluepuma77]

Why not roll back the change? SvelteKit has many adapters, it seems only relevant for one.

For adapter-static it is only the case (#4441) when someone wrongly set prerendering.enabled = false.

[dimfeld]

Using prerendering.enabled: false with the static adapter and a fallback file is a valid configuration when making a "SPA mode" app. It is not "wrong" to configure it that way and some of us are saddled with legacy code or dependencies that simply will not run in modern ESM-mode Node.js, or facing other compatibility issues such as #4261 which don't always present a workaround right now.

I guarantee there's a way to fix this that results in working builds for everyone; let's work on that.

[dimfeld]

Ok, I believe I found a workaround for my case. It's a little weird but appears to work.

Setting prerender.enabled: true and prerender.entries: [] bypasses all the code that tries to actually prerender stuff in a way that omitting entries and setting prerender.default: false does not, while still creating the fallback file.

With this workaround, I think we can perhaps go back to the 303 version prerender code, but this feels very fragile so I would still prefer a future change that would break out fallback generation from starting a Server at all, or something similar.

[mrkishi]

Is the fallback html truly only used for SPA mode or is it used on some other condition?

If it's really only used for SPAs, it does sound like we should render the fallback differently by not running a full SSR but instead just filling app.html with a blank skeleton when prerender.enabled = false.

[dimfeld]

I think it's only for SPA mode but I'm not 100% sure on that.

Your suggested solution is basically what I was thinking too, since it doesn't appear to be doing anything that actually needs to start the Server and instead just needs to share a couple bits of code from the SSR render pipeline. I'm saying that without really knowing what's going on under the hood though.

The current behavior is to take the app.html, and do an empty SSR render whose body just inserts the <script> tag to download the main JS file and call start.

Looking at the output on my project at work, I see these replacements:

%svelte.head% becomes

    <meta http-equiv="content-security-policy" content="">
	<link rel="modulepreload" href="/_app/start-75b9d96b.js">
	<link rel="modulepreload" href="/_app/chunks/index-7a365041.js">
	<link rel="modulepreload" href="/_app/chunks/index-d3785d77.js">
	<link rel="modulepreload" href="/_app/chunks/preload-helper-e4860ae8.js">
	<link rel="modulepreload" href="/_app/chunks/singletons-d1fb5791.js">

And then %svelte.body% is replaced by

		<script type="module" data-hydrate="45h">
		import { start } from "/_app/start-75b9d96b.js";
		start({
			target: document.querySelector('[data-hydrate="45h"]').parentNode,
			paths: {"base":"","assets":""},
			session: {},
			route: true,
			spa: true,
			trailing_slash: "never",
			hydrate: null
		});
	</script>

[coryvirok]

FWIW, I ran into this today as well but I'm using the node adaptor. I don't have any prerenderable routes, (I've never set the prerender module export or set the default to true to attempt to prerender everything.)

The code I have in hooks.ts is pretty simple. I simply look for a required server env var and throw an exception if it's not available.

const envKey = process.env.ENV_KEY
if(!envKey) {
        throw new Error('missing ENV_KEY')
}

I do this in order to fast-fail the server startup.

The reason I'm writing all of this is because there's probably a better hook for this code, e.g. something like this in hooks.ts:

export const onStartup = async (...) => {
    // my code
};

Anything like this on the radar? It would solve this code execution during build problem for this use-case.

Thanks!

[bluepuma77]

@Rich-Harris @benmccann We have a real blocker here for many adapter-node users. We can't build our SvelteKit app.

Sure, you can argue that we can just sit on the last working version, but that means we are also staying with the other bugs that have been fixed in the mean time with later releases.

If this is "just" for adapter-static and "just" for a fallback file (emphasis is mine, sorry) and @dimfeld has a work around for his situation, how about we roll back the change so that adapter-node users can build their SvelteKit applications again?

[dimfeld]

I had an idea in dimfeld@5d1beb1 to try to run the fallback generation without loading hooks but haven't had the time to really try it yet.

[PH4NTOMiki]

@dimfeld that is not really a viable option(running without hooks), because i'm using adapter-static and I have default session set similar to this: {user:{id:null, firstName:null}} and in Svelte components I use something like this: {#if $session.user.id != null} and that would break if you did that, because the getSession wouln't run and session would be {} and that wouldn't work.
And I'm also sure there are other cases when running handle hook is necessary(even for adapter-static), and that would break too.

The best solution(temporary) is to somehow emulate the behavior in your work-around(empty entries)

[dimfeld]

Please look at the change itself; that is not what’s happening. It renders only the fallback without hooks, not the entire app. Anyway I haven’t been able it try it out yet to see if it actually works. If anyone else would like to, please feel free

…

On Apr 10, 2022, 5:07 AM -1000, Mihael Mutić ***@***.***>, wrote: @dimfeld that is not really a viable option(running without hooks), because i'm using adapter-static and I have default session set similar to this: {user:{id:null, firstName:null}} and in Svelte components I use something like this: {#if $session.user.id != null} and that would break if you did that, because the getSession wouln't run and session would be {} and that wouldn't work. And I'm also sure there are other cases when running handle hook is necessary(even for adapter-static), and that would break too. The best solution(temporary) is to somehow emulate the behavior in your work-around(empty entries) — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[PH4NTOMiki]

@dimfeld you don't understand me, I need hooks(getSession at least, and some may even need handle hook) to run even for generating fallback file, because I need default session to bet set for everything to work

[dimfeld]

I do understand you. I don’t think you understand how the fallback file is generated. It doesn’t load any pages or layouts.

…

On Apr 10, 2022, 6:07 AM -1000, Mihael Mutić ***@***.***>, wrote: @dimfeld you don't understand me, I need hooks(getSession at least, and some may even need handle hook) to run even for generating fallback file, because I need default session to bet set for everything to work — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[PH4NTOMiki]

No you don't, and you don't understand how fallback is generated. I know it doesn't contain layout or page, but it DOES contain <script> tag with start function invocation, and one of the parameters of that function is session and that parameter gets filled out by getSession hook in here

kit/packages/kit/src/runtime/server/page/render.js

Lines 163 to 165 in 1c0b6b6

	session: ${try_serialize($session, (error) => {
	throw new Error(`Failed to serialize session data: ${error.message}`);
	})},

either way(fallback or not, that needs to get filled out to be able to start SvelteKit client app)

[PH4NTOMiki]

For fallback its here

kit/packages/kit/src/runtime/server/index.js

Lines 179 to 184 in 32a700a

	if (state.prerender && state.prerender.fallback) {
	return await render_response({
	event,
	options,
	state,
	$session: await options.hooks.getSession(event),

[dimfeld]

Apologies, I had just woken up and wasn’t properly thinking things through. Of course the session would need to be seeded even in fallback mode.  Sorry for the curt response earlier.

…

On Apr 10, 2022, 6:30 AM -1000, Mihael Mutić ***@***.***>, wrote: For fallback its here https://github.com/sveltejs/kit/blob/32a700a204e108a1fdceeffc7727bdb340335a3e/packages/kit/src/runtime/server/index.js#L179-L184 — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[PH4NTOMiki]

No worries, I reacted a bit harsh too.

[dimfeld]

So at this point I think that the following changes should be sufficient:

• Revert the previous PR that changed the fallback render behavior
• Mention in the SPA Mode section of the adapter-static docs that setting prerender.entries: [] can help if Node.js throws errors when importing your page/layout components.

If I have some time tonight I'll try to get a PR together for this.

[ximus]

thanks @dimfeld for carrying this through

[jerrygreen]

When setting prerender.entries: [] in config, it helps to skip the problem, but then the whole sense of adapter-static is lost, it seems... At least, upon building with prerender.entries: [], I'm not seeing all my static pages in build folder, and say... Can't use it for Github Pages.

Maybe there's actual solution, rather than skipping the problem?

P.S. I used the most currently recent "@sveltejs/kit": "1.0.0-next.405" and "@sveltejs/adapter-static": "1.0.0-next.39".

[Rich-Harris]

hey all — flagging this discussion, as people here might have thoughts on it: #7716