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

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

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Reactions: 10
  • Comments: 33 (24 by maintainers)

Commits related to this issue

Most upvoted comments

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.

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

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!

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.

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.

@dimfeld Hi, I’ve been beating my head against the wall for around three hours now because of this. My Docker builds were failing and the error I was getting was about my API endpoints not being available, which I thought was the weirdest thing ever. After hours of debugging and googling I found this issue.

Obviously whatever change has been made, should’ve only been made for adapter-static, I’m using adapter-node and I want nothing to do with any of this.

@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.

@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?

…when using the static adapter…

@dimfeld Well, then why not do this only “when using the static adapter”?! It’s patently obvious that this shouldn’t happen for other adapters, such as adapter-node, it breaks everything.

Same here, I’m encountering this exact problem. The new behavior is weird and unexpected, why would you execute application code and by extension trigger API calls/database connections during build?! That could screw up absolutely everything.

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

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.

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.

Because we still need a fallback file when using the static adapter with prerendering disabled

Then shouldn’t this only be done “when using the static adapter with prerendering disabled”? i’m using node adapter with default settings and this is suddenly breaking my builds.

I run CI production builds of my app that are not setup to handle running the web server, handle requests or do any sort of pre-rending, as sadly I have server initialization logic that expects certain external services to be running, which aren’t in build.

Generating this fallback has broken my builds.

I’m going to investigate making changes to my app so that it can at least handle this fallback request. Would be nice if we can find a way to avoid it altogether.

I see adapter-static being mentioned as a reason for this change, but I only use adapter-node I wish I could suggest a fix or a workaround but I don’t have enough context.