feat: Move hooks.{js|ts} init to Server.init instead of Server.respond (#6179)

elliott-with-the-longest-name-on-github · Rich-Harris · web-flow · commit 64449a7138b9 · 2022-08-24T10:59:37.000-04:00
* feat: Move hooks.js initialization into Server.init

* fix: Server type

* feat: Await Server.init in adapters

* feat: Await server.init in preview

* fix: Init server during prerender

* feat: Init hooks when dev mode starts instead of on first request

* fix: Prerendering, but better

* feat: Test env vars in prerender

* fix: Remove console.log

* fix: Import in hooks.js of basics test app

* changeset

* Update packages/adapter-netlify/src/serverless.js

Co-authored-by: Rich Harris &lt;richard.a.harris@gmail.com&gt;

* Update packages/kit/src/vite/build/build_server.js

Co-authored-by: Rich Harris &lt;richard.a.harris@gmail.com&gt;

* feat: Enable top-level await in `adapter-node`

* feat: Add warning comment for future devs

* fix: Init env prior to hooks

* fix: Remove dear Rich

* fix: Apparently didn't like my tsconfig

* fix: Move env and hooks initialization later

* move user_hooks init back from whence it came

Co-authored-by: Rich Harris &lt;richard.a.harris@gmail.com&gt;
Co-authored-by: Rich Harris &lt;hello@rich-harris.dev&gt;
diff --git a/.changeset/curvy-students-march.md b/.changeset/curvy-students-march.md
@@ -0,0 +1,10 @@
+---
+'@sveltejs/adapter-cloudflare': patch
+'@sveltejs/adapter-cloudflare-workers': patch
+'@sveltejs/adapter-netlify': patch
+'@sveltejs/adapter-node': patch
+'@sveltejs/adapter-vercel': patch
+'@sveltejs/kit': patch
+---
+
+[feat] Moved hooks.js initialization from Server.respond into Server.init
diff --git a/packages/adapter-cloudflare-workers/files/entry.js b/packages/adapter-cloudflare-workers/files/entry.js
@@ -15,7 +15,7 @@ export default {
 	 * @param {any} context
 	 */
 	async fetch(req, env, context) {
-		server.init({ env });
+		await server.init({ env });
 
 		const url = new URL(req.url);
 
diff --git a/packages/adapter-cloudflare/src/worker.js b/packages/adapter-cloudflare/src/worker.js
@@ -9,7 +9,7 @@ const prefix = `/${manifest.appDir}/`;
 /** @type {import('worktop/cfw').Module.Worker<{ ASSETS: import('worktop/cfw.durable').Durable.Object }>} */
 const worker = {
 	async fetch(req, env, context) {
-		server.init({ env });
+		await server.init({ env });
 		// skip cache if "cache-control: no-cache" in request
 		let pragma = req.headers.get('cache-control') || '';
 		let res = !pragma.includes('no-cache') && (await Cache.lookup(req));
diff --git a/packages/adapter-netlify/src/serverless.js b/packages/adapter-netlify/src/serverless.js
@@ -9,11 +9,16 @@ import { split_headers } from './headers';
 export function init(manifest) {
 	const server = new Server(manifest);
 
-	server.init({
+	let init_promise = server.init({
 		env: process.env
 	});
 
 	return async (event, context) => {
+		if (init_promise !== null) {
+			await init_promise;
+			init_promise = null;
+		}
+
 		const response = await server.respond(to_request(event), {
 			platform: { context },
 			getClientAddress() {
diff --git a/packages/adapter-node/src/handler.js b/packages/adapter-node/src/handler.js
@@ -11,7 +11,7 @@ import { env } from './env.js';
 /* global ENV_PREFIX */
 
 const server = new Server(manifest);
-server.init({ env: process.env });
+await server.init({ env: process.env });
 const origin = env('ORIGIN', undefined);
 const xff_depth = parseInt(env('XFF_DEPTH', '1'));
 
diff --git a/packages/adapter-node/tsconfig.json b/packages/adapter-node/tsconfig.json
@@ -6,7 +6,8 @@
 		"noImplicitAny": true,
 		"allowSyntheticDefaultImports": true,
 		"moduleResolution": "node",
-		"module": "es2020",
+		"module": "es2022",
+		"target": "es2017",
 		"baseUrl": ".",
 		"paths": {
 			"@sveltejs/kit": ["../kit/types/index"]
diff --git a/packages/adapter-vercel/files/serverless.js b/packages/adapter-vercel/files/serverless.js
@@ -7,7 +7,7 @@ installPolyfills();
 
 const server = new Server(manifest);
 
-server.init({
+await server.init({
 	env: process.env
 });
 
diff --git a/packages/kit/src/core/prerender/prerender.js b/packages/kit/src/core/prerender/prerender.js
@@ -15,7 +15,7 @@ import { load_config } from '../config/index.js';
  * @typedef {import('types').Logger} Logger
  */
 
-const [, , client_out_dir, results_path, manifest_path, verbose] = process.argv;
+const [, , client_out_dir, results_path, manifest_path, verbose, env] = process.argv;
 
 prerender();
 
@@ -159,6 +159,7 @@ export async function prerender() {
 	});
 
 	const server = new Server(manifest);
+	await server.init({ env: JSON.parse(env) });
 
 	const error = normalise_error_handler(log, config);
 
diff --git a/packages/kit/src/vite/build/build_server.js b/packages/kit/src/vite/build/build_server.js
@@ -84,7 +84,12 @@ export class Server {
 		};
 	}
 
-	init({ env }) {
+	/**
+	 * Take care: Some adapters may have to call \`Server.init\` per-request to set env vars,
+	 * so anything that shouldn't be rerun should be wrapped in an \`if\` block to make sure it hasn't
+	 * been done already.
+	 */
+	async init({ env }) {
 		const entries = Object.entries(env);
 
 		const prv = Object.fromEntries(entries.filter(([k]) => !k.startsWith('${
@@ -99,12 +104,6 @@ export class Server {
 		set_public_env(pub);
 
 		this.options.public_env = pub;
-	}
-
-	async respond(request, options = {}) {
-		if (!(request instanceof Request)) {
-			throw new Error('The first argument to server.respond must be a Request object. See https://github.com/sveltejs/kit/pull/3384 for details');
-		}
 
 		if (!this.options.hooks) {
 			const module = await import(${s(hooks)});
@@ -114,6 +113,12 @@ export class Server {
 				externalFetch: module.externalFetch || fetch
 			};
 		}
+	}
+
+	async respond(request, options = {}) {
+		if (!(request instanceof Request)) {
+			throw new Error('The first argument to server.respond must be a Request object. See https://github.com/sveltejs/kit/pull/3384 for details');
+		}
 
 		return respond(request, this.options, options);
 	}
diff --git a/packages/kit/src/vite/index.js b/packages/kit/src/vite/index.js
@@ -411,7 +411,13 @@ function kit() {
 
 				const child = fork(
 					script,
-					[vite_config.build.outDir, results_path, manifest_path, '' + verbose],
+					[
+						vite_config.build.outDir,
+						results_path,
+						manifest_path,
+						'' + verbose,
+						JSON.stringify({ ...env.private, ...env.public })
+					],
 					{
 						stdio: 'inherit'
 					}
diff --git a/packages/kit/src/vite/preview/index.js b/packages/kit/src/vite/preview/index.js
@@ -45,7 +45,7 @@ export async function preview(vite, vite_config, svelte_config) {
 	});
 
 	const server = new Server(manifest);
-	server.init({
+	await server.init({
 		env: loadEnv(vite_config.mode, process.cwd(), '')
 	});
 
diff --git a/packages/kit/test/apps/basics/src/hooks.js b/packages/kit/test/apps/basics/src/hooks.js
@@ -1,6 +1,6 @@
 import fs from 'fs';
 import cookie from 'cookie';
-import { sequence } from '../../../../src/hooks';
+import { sequence } from '@sveltejs/kit/hooks';
 
 /** @type {import('@sveltejs/kit').HandleError} */
 export const handleError = ({ event, error }) => {
diff --git a/packages/kit/test/prerendering/basics/.env b/packages/kit/test/prerendering/basics/.env
@@ -0,0 +1,5 @@
+PRIVATE_STATIC="accessible to server-side code/replaced at build time"
+PRIVATE_DYNAMIC="accessible to server-side code/evaluated at run time"
+
+PUBLIC_STATIC="accessible anywhere/replaced at build time"
+PUBLIC_DYNAMIC="accessible anywhere/evaluated at run time"
diff --git a/packages/kit/test/prerendering/basics/.gitignore b/packages/kit/test/prerendering/basics/.gitignore
@@ -0,0 +1 @@
+!.env
diff --git a/packages/kit/test/prerendering/basics/src/routes/env/+page.server.js b/packages/kit/test/prerendering/basics/src/routes/env/+page.server.js
@@ -0,0 +1,9 @@
+import { PRIVATE_STATIC } from '$env/static/private';
+import { env } from '$env/dynamic/private';
+
+export function load() {
+	return {
+		PRIVATE_STATIC,
+		PRIVATE_DYNAMIC: env.PRIVATE_DYNAMIC
+	};
+}
diff --git a/packages/kit/test/prerendering/basics/src/routes/env/+page.svelte b/packages/kit/test/prerendering/basics/src/routes/env/+page.svelte
@@ -0,0 +1,13 @@
+<script>
+	import { PUBLIC_STATIC } from '$env/static/public';
+	import { env } from '$env/dynamic/public';
+
+	/** @type {import('./$types').PageData} */
+	export let data;
+</script>
+
+<p id="static-private">PRIVATE_STATIC: {data.PRIVATE_STATIC}</p>
+<p id="dynamic-private">PRIVATE_DYNAMIC: {data.PRIVATE_DYNAMIC}</p>
+
+<p id="static-public">PUBLIC_STATIC: {PUBLIC_STATIC}</p>
+<p id="dynamic-public">PUBLIC_DYNAMIC: {env.PUBLIC_DYNAMIC}</p>
diff --git a/packages/kit/test/prerendering/basics/test/test.js b/packages/kit/test/prerendering/basics/test/test.js
@@ -190,4 +190,19 @@ test('respects config.prerender.origin', () => {
 	assert.ok(content.includes('<h2>http://example.com</h2>'));
 });
 
+test('$env - includes environment variables', () => {
+	const content = read('env.html');
+
+	assert.match(
+		content,
+		/.*PRIVATE_STATIC: accessible to server-side code\/replaced at build time.*/gs
+	);
+	assert.match(
+		content,
+		/.*PRIVATE_DYNAMIC: accessible to server-side code\/evaluated at run time.*/gs
+	);
+	assert.match(content, /.*PUBLIC_STATIC: accessible anywhere\/replaced at build time.*/gs);
+	assert.match(content, /.*PUBLIC_DYNAMIC: accessible anywhere\/evaluated at run time.*/gs);
+});
+
 test.run();
diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts
@@ -264,7 +264,7 @@ export interface ResolveOptions {
 
 export class Server {
 	constructor(manifest: SSRManifest);
-	init(options: ServerInitOptions): void;
+	init(options: ServerInitOptions): Promise<void>;
 	respond(request: Request, options: RequestOptions): Promise<Response>;
 }
 
diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts
@@ -94,7 +94,7 @@ export interface ImportNode {
 }
 
 export class InternalServer extends Server {
-	init(options: ServerInitOptions): void;
+	init(options: ServerInitOptions): Promise<void>;
 	respond(
 		request: Request,
 		options: RequestOptions & {

Original file line number	Diff line number	Diff line change
`@@ -411,7 +411,13 @@ function kit() {`
`411`	`411`
`412`	`412`	`const child = fork(`
`413`	`413`	`script,`
`414`		`- [vite_config.build.outDir, results_path, manifest_path, '' + verbose],`
	`414`	`+ [`
	`415`	`+ vite_config.build.outDir,`
	`416`	`+ results_path,`
	`417`	`+ manifest_path,`
	`418`	`+ '' + verbose,`
	`419`	`+ JSON.stringify({ ...env.private, ...env.public })`
	`420`	`+ ],`
`415`	`421`	`{`
`416`	`422`	`stdio: 'inherit'`
`417`	`423`	`}`