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

Unable to entirely disable prerendering #7899

Closed
georgecrawford opened this issue Dec 1, 2022 · 13 comments · Fixed by #8013
Closed

Unable to entirely disable prerendering #7899

georgecrawford opened this issue Dec 1, 2022 · 13 comments · Fixed by #8013
Milestone

Comments

@georgecrawford
Copy link
Contributor

Describe the bug

Hi,

Forgive me if I've missed something! I'd like to run SvelteKit as a SSR-and-CSR solution, with no build-time prerendering.

From the docs, I can disable prerendering routes by adding a export const prerender = false; annotation to the top-level +layout.js. This seems to work.

I can also prevent the build process from crawling for entries to prerender (svelte.config.js):

        // Disable prerendering, as this is a dynamic app
        prerender: {
            crawl: false,
            entries: [],
        },

However, despite these two settings, I am still seeing an attempt to prerender when I run vite build:

#13 38.79 handleFeatureFlags (hooks/featureFlags.js) running for URL: http://sveltekit-prerender/[fallback]
...
#13 58.82 TypeError: Cannot destructure property 'CI_ENVIRONMENT_NAME' of 'env' as it is undefined.
#13 58.82     at reportError (file:///app/server/.svelte-kit/output/server/chunks/hooks.server.js:954:32)
#13 58.82     at Object.handleError (file:///app/server/.svelte-kit/output/server/chunks/hooks.server.js:1011:27)
#13 58.82     at Object.handle_error (file:///app/server/.svelte-kit/output/server/index.js:2369:35)
#13 58.82     at handle_error_and_jsonify (file:///app/server/.svelte-kit/output/server/index.js:243:20)
#13 58.82     at handle_fatal_error (file:///app/server/.svelte-kit/output/server/index.js:226:22)
#13 58.82     at respond (file:///app/server/.svelte-kit/output/server/index.js:2196:18)
#13 58.82     at async prerender (file:///app/server/node_modules/@sveltejs/kit/src/core/prerender/prerender.js:439:19)
...
#13 58.85 [vite-plugin-svelte-kit] Prerendering failed with code 1
#13 58.85 error during build:
#13 58.85 Error: Prerendering failed with code 1
#13 58.85     at ChildProcess.<anonymous> (file:///app/server/node_modules/@sveltejs/kit/src/exports/vite/index.js:462:15)
#13 58.85     at ChildProcess.emit (node:events:513:28)
#13 58.85     at ChildProcess.emit (node:domain:489:12)
#13 58.85     at Process.ChildProcess._handle.onexit (node:internal/child_process:293:12)

I've tracked this down to this code block:

	const rendered = await server.respond(new Request(config.prerender.origin + '/[fallback]'), {
		getClientAddress,
		prerendering: {
			fallback: true,
			dependencies: new Map()
		}
	});

	const file = `${config.outDir}/output/prerendered/fallback.html`;
	mkdirp(dirname(file));
	writeFileSync(file, await rendered.text());

Whereas prerendering known routes and other entries can be disabled with the above config, there doesn't seem to be a way to disable the rendering of the fallback.html file. This is a problem for anyone who wants to ship code in server hooks which doesn't work in the environment in which vite build is run.

Would it be possible to allow this fallback prerendering to be disabled? If not, it'd be great to understand why it's important (somewhere in the documentation), and I guess the only option is for the hooks to make use of $app/environment.building or similar to only conditionally run their logic?

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-x9vwcp

This project adds the following changes to the default https://node.new/sveltekit:

  • adds export const prerender = false; to src/routes/+layout.js
  • removes export const prerender = true; from all other routes
  • disables prerender crawling in svelte.config.js:
const config = {
	kit: {
		adapter: adapter(),

		// Disable prerendering, as this is a dynamic app
		prerender: {
			crawl: false,
			entries: []
		}
	}
};
  • adds a hooks.server.js file which relies on environment variables:
export async function handle({ event, resolve }) {
	const thisWillFail = new URL(process.env.UNDECLARED_ENV_VAR_DURING_BUILD);

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

export async function handleError({ error, event }) {
	const thisWillFail = new URL(process.env.UNDECLARED_ENV_VAR_DURING_BUILD);

	return error;
}
  • modifies the npm run dev script to be "UNDECLARED_ENV_VAR_DURING_BUILD='http://testing' vite dev" so that local dev works

Running npm run build shows that an attempt to prerender the URL http://sveltekit-prerender/[fallback] is still made, and since the environment variable is not declared, the hooks cause the build to fail:

...
.svelte-kit/output/server/chunks/hooks.server.js                0.37 KiB
handle hook running for URL: http://sveltekit-prerender/[fallback]
TypeError [ERR_INVALID_URL]: Invalid URL
...
  input: 'undefined',
  code: 'ERR_INVALID_URL'
}
[vite-plugin-svelte-kit] Prerendering failed with code 1
error during build:
Error: Prerendering failed with code 1
...

One solution I've found for this is the following, in hooks.server.js:

import {building} from '$app/environment';

export const handle = building
    ? sequence()
    : sequence(
          handleLoggingAndMonitoring,
          handleCookieParsing,
          handleFeatureFlags,
          handleEnvVars,
          handleImageUrls,
          handleApiRequest,
          handleApiServices,
          handleSecurityHeaders
      );

Is this the recommended approach? If so, it still seems odd to me that I'm unable to disable prerendering across the board.

Logs

No response

System Info

System:
    OS: macOS 12.4
    CPU: (8) arm64 Apple M1 Pro
    Memory: 212.80 MB / 32.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 16.18.0 - /usr/local/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 8.19.2 - /usr/local/bin/npm
    Watchman: 2022.11.14.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 107.0.5304.110
    Chrome Canary: 110.0.5443.0
    Firefox: 105.0.3
    Safari: 15.5
  npmPackages:
    @sveltejs/adapter-node: ^1.0.0-next.101 => 1.0.0-next.101
    @sveltejs/kit: ^1.0.0-next.570 => 1.0.0-next.570
    svelte: ^3.53.1 => 3.53.1

Severity

blocking all usage of SvelteKit

Additional Information

No response

@dummdidumm
Copy link
Member

Could you try setting the prerender.entries option to [] ?

@Rich-Harris
Copy link
Member

This is expected — it's a result of this change, which ensures that SvelteKit can analyse your code in order to configure deployments in future.

That said, importing and analysing your code is one thing; actually rendering stuff (the fallback) is something else, since it means having to do if (!building) in more places. I'm not immediately sure what a good solution is but we should probably come up with something.

By the way, it's better to import env vars from $env/static/private or $env/dynamic/private than to use process.env, since it's cross-platform, type-safe, and in the static case will give you constant folding and dead code elimination etc.

@Rich-Harris
Copy link
Member

(To expand: the reason this is unfortunate is that the fallback page is always generated but only rarely used. There's a tricky order-of-operations thing — if you're using adapter-static and specify a fallback: '200.html' then the fallback page already needs to exist. I guess we could add a generateFallback() method inside adapters that restarted the process in order to generate the fallback — adds complexity in the case where you do need it, but otherwise contains it.)

@georgecrawford
Copy link
Contributor Author

Thanks for your thoughts, both. I'm aware of the $env namespace, but I don't currently have the time to migrate over - we're up against the wire trying to upgrade to the RC of SvelteKit and just 'make things work'. The single workaround above (disabling our hooks when building is true) works for the time being, and I'll reference this ticket to see if anything more elegant appears in the codebase in future.

I want to reiterate my thanks to you both (@Rich-Harris and @dummdidumm) and the wider team. Kit is a fantastic piece of work, and your recent breaking changes are so much better for ergonomics. Well done for being brave and sticking to doing the best you can for v1. Exciting days are very close now.

@jwatts777
Copy link

@georgecrawford I was the lost soul trying to help you on Discord.
Just wanted to say thanks for posting this, I learned quite a lot about an area of Sveltekit I've never touched.

@Rich-Harris Rich-Harris added this to the 1.0 milestone Dec 2, 2022
@schibrikov
Copy link

I have also faced this issue since I'm making API calls in server hooks.
I was reeeally surprised when my build has failed because of the backend API not being available at build time. This is such a strange dependency, feels like CI shouldn't rely on runtime stuff.

@aradalvand
Copy link
Contributor

aradalvand commented Dec 7, 2022

Same. There should definitely be some sort of a global option to let users opt out of everything prerendering-related entirely. This area is still confusing.

@dimfeld
Copy link
Contributor

dimfeld commented Dec 8, 2022

It looks like #7878 released in version 573 also made this issue more salient. We still have a lot of legacy code in our app that uses Angular 1 (aka AngularJS), and just importing angular uses window. We're using adapter-static and entries: [] to disable prerendering as much as possible.

Until version 573, this worked fine since the prerendering code was surrounded with try/catch which would basically ignore any errors. But at https://github.com/sveltejs/kit/pull/7878/files#diff-640bb06029f8b528408cb17e6b0a88cd9d68924246d520db4a616a8a2fe7d4f9L384 this try/catch was removed, and so our app no longer builds.

I confirmed that editing the file to add back that try/catch does allow the app to build, but of course that's not a proper solution since in the "normal" case you do want to throw those errors.

Note that this is a rather different cause from OP's problem, since we're not doing anything with hooks -- it's the pages themselves and their dependencies that import angular. But I believe that a way to totally disable prerendering would also fix this issue. For now we're fine staying on 572.

@Rich-Harris
Copy link
Member

@dimfeld Are you importing angular in +page.js (and similar) files? Aside from the fallback issue (which would be fixed with a builder.generateFallback(dest) API), your +page.svelte and +layout.svelte components wouldn't be imported during build (unless a page is discovered to be prerenderable, and reachable via entries), so if that's where angular is imported then things should work fine.

If you are importing angular in .js files, then I'd expect that to blow up at runtime, in which case erroring during building seems like a positive change. Very possible that I've misunderstood the scenario though.

@dimfeld
Copy link
Contributor

dimfeld commented Dec 9, 2022

Indirectly, yes, by importing other packages in our monorepo which in turn import angular.

But we're not running anything Svelte-related on the server though -- we build with adapter-static and just serve the JS/CSS/HTML directly from nginx. The app has been running in production with the SvelteKit/Angular hybrid since last year and managed the big routing change just fine, so I can confirm it does work at runtime :)

But that's a good point though... Most of our usage is in the .svelte files. There are only a couple places in the actual .js files that import the offending package, and those places are only importing a single function, so we could perhaps work around that by just deep importing the one file with that function.

@Rich-Harris
Copy link
Member

Interesting — I don't quite understand how you're able to build the site with adapter-static if there are are parts of your app that depend on an unimportable module?

@dimfeld
Copy link
Contributor

dimfeld commented Dec 9, 2022

We're running a "SPA-mode" setup. Prerendering disabled as much as we can with prerender.entries = [], a fallback page to run the app, nginx set up to set the fallback page in the normal way for such things, and whatever else is necessary to make that work. I think the angular import errors were still there before; they just didn't matter because the try/catch statement was swallowing them.

Anyway, I think the details are moot for now, since I just confirmed that it does work to deep-import the specific files that we need so that angular doesn't get imported from the .js files. I think I'm good now. Thanks!

It's been a long process getting Angular slowly stripped out of our 8-year-old app, can't wait to get it finished and be 100% on SvelteKit :)

@georgecrawford
Copy link
Contributor Author

Thanks again, @Rich-Harris and @dummdidumm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants