set public env in <script>, implementing preloading on safari (#9018)

* load public env lazily * not sure why this fixes it but hey * fix tests * fix test * eager imports * use a hacky polyfill instead of eagerly importing * hash things up a bit * use header/element combo * remove hack * add an explanatory comment * remove TODO * implement generatePublicEnv * fix test * add a clarifying comment * fix tests * Update packages/adapter-static/test/test.js * ugh let this be the last one * always inline public env * fix * remove unused re-export * lint * lint * tidy up * add missing .env file
sveltejs · Feb 16, 2023 · dd4a45b · dd4a45b
1 parent 1cb42d4
commit dd4a45b
Show file tree

Hide file tree

Showing 16 changed files with 92 additions and 72 deletions.
diff --git a/packages/adapter-static/test/apps/prerendered/.env b/packages/adapter-static/test/apps/prerendered/.env
@@ -0,0 +1 @@
+PUBLIC_ANSWER=42
diff --git a/packages/adapter-static/test/apps/prerendered/.gitignore b/packages/adapter-static/test/apps/prerendered/.gitignore
@@ -3,3 +3,4 @@ node_modules
 /.svelte-kit
 /build
 /functions
+!/.env
diff --git a/packages/adapter-static/test/apps/prerendered/src/routes/public-env/+page.svelte b/packages/adapter-static/test/apps/prerendered/src/routes/public-env/+page.svelte
@@ -0,0 +1,5 @@
+<script>
+	import { env } from '$env/dynamic/public';
+</script>
+
+<h1>The answer is {env.PUBLIC_ANSWER}</h1>
diff --git a/packages/adapter-static/test/test.js b/packages/adapter-static/test/test.js
@@ -20,6 +20,11 @@ run('prerendered', (test) => {
 	test('prerenders a referenced endpoint with implicit `prerender` setting', async ({ cwd }) => {
 		assert.ok(fs.existsSync(`${cwd}/build/endpoint/implicit.json`));
 	});
+
+	test('exposes public env vars to the client', async ({ cwd, base, page }) => {
+		await page.goto(`${base}/public-env`);
+		assert.equal(await page.textContent('h1'), 'The answer is 42');
+	});
 });
 
 run('spa', (test) => {

diff --git a/packages/kit/src/core/env.js b/packages/kit/src/core/env.js
@@ -3,11 +3,6 @@ import { runtime_base } from './utils.js';
 
 /**
  * @typedef {'public' | 'private'} EnvType
- * @typedef {{
- * 	public: Record<string, string>;
- * 	private: Record<string, string>;
- * 	prefix: string;
- * }} EnvData
  */
 
 /**
@@ -49,7 +44,7 @@ export function create_dynamic_module(type, dev_values) {
 
 /**
  * @param {EnvType} id
- * @param {EnvData} env
+ * @param {import('types').Env} env
  * @returns {string}
  */
 export function create_static_types(id, env) {
@@ -63,15 +58,16 @@ export function create_static_types(id, env) {
 
 /**
  * @param {EnvType} id
- * @param {EnvData} env
+ * @param {import('types').Env} env
+ * @param {string} prefix
  * @returns {string}
  */
-export function create_dynamic_types(id, env) {
+export function create_dynamic_types(id, env, prefix) {
 	const properties = Object.keys(env[id])
 		.filter((k) => valid_identifier.test(k))
 		.map((k) => `\t\t${k}: string;`);
 
-	const prefixed = `[key: \`${env.prefix}\${string}\`]`;
+	const prefixed = `[key: \`${prefix}\${string}\`]`;
 
 	if (id === 'private') {
 		properties.push(`\t\t${prefixed}: undefined;`);

diff --git a/packages/kit/src/core/sync/write_ambient.js b/packages/kit/src/core/sync/write_ambient.js
@@ -21,9 +21,10 @@ function read_description(filename) {
 }
 
 /**
- * @param {import('../env.js').EnvData} env
+ * @param {import('types').Env} env
+ * @param {string} prefix
  */
-const template = (env) => `
+const template = (env, prefix) => `
 ${GENERATED_COMMENT}
 
 /// <reference types="@sveltejs/kit" />
@@ -35,10 +36,10 @@ ${read_description('$env+static+public.md')}
 ${create_static_types('public', env)}
 
 ${read_description('$env+dynamic+private.md')}
-${create_dynamic_types('private', env)}
+${create_dynamic_types('private', env, prefix)}
 
 ${read_description('$env+dynamic+public.md')}
-${create_dynamic_types('public', env)}
+${create_dynamic_types('public', env, prefix)}
 `;
 
 /**
@@ -53,6 +54,6 @@ export function write_ambient(config, mode) {
 
 	write_if_changed(
 		path.join(config.outDir, 'ambient.d.ts'),
-		template({ ...env, prefix: config.env.publicPrefix })
+		template(env, config.env.publicPrefix)
 	);
 }
diff --git a/packages/kit/src/core/sync/write_server.js b/packages/kit/src/core/sync/write_server.js
@@ -1,5 +1,6 @@
 import fs from 'node:fs';
 import path from 'node:path';
+import { hash } from '../../runtime/hash.js';
 import { posixify, resolve_entry } from '../../utils/filesystem.js';
 import { s } from '../../utils/misc.js';
 import { load_error_page, load_template } from '../config/index.js';
@@ -30,6 +31,7 @@ import { set_assets, set_building, set_private_env, set_public_env, set_version
 set_version(${s(config.kit.version.name)});
 
 export const options = {
+	app_template_contains_nonce: ${template.includes('%sveltekit.nonce%')},
 	csp: ${s(config.kit.csp)},
 	csrf_check_origin: ${s(config.kit.csrf.checkOrigin)},
 	embedded: ${config.kit.embedded},
@@ -50,7 +52,8 @@ export const options = {
 		error: ({ status, message }) => ${s(error_page)
 			.replace(/%sveltekit\.status%/g, '" + status + "')
 			.replace(/%sveltekit\.error\.message%/g, '" + message + "')}
-	}
+	},
+	version_hash: ${s(hash(config.kit.version.name))}
 };
 
 export function get_hooks() {

diff --git a/packages/kit/src/exports/vite/index.js b/packages/kit/src/exports/vite/index.js
@@ -23,6 +23,7 @@ import { write_client_manifest } from '../../core/sync/write_client_manifest.js'
 import prerender from '../../core/postbuild/prerender.js';
 import analyse from '../../core/postbuild/analyse.js';
 import { s } from '../../utils/misc.js';
+import { hash } from '../../runtime/hash.js';
 
 export { vitePreprocess } from '@sveltejs/vite-plugin-svelte';
 
@@ -164,6 +165,8 @@ function kit({ svelte_config }) {
 	const { kit } = svelte_config;
 	const out = `${kit.outDir}/output`;
 
+	const version_hash = hash(kit.version.name);
+
 	/** @type {import('vite').ResolvedConfig} */
 	let vite_config;
 
@@ -351,6 +354,12 @@ function kit({ svelte_config }) {
 						vite_config_env.command === 'serve' ? env.private : undefined
 					);
 				case '\0$env/dynamic/public':
+					// populate `$env/dynamic/public` from `window` in the case where
+					// modules are externally hosted
+					if (is_build && !options?.ssr) {
+						return `export const env = window.__sveltekit_${version_hash}.env;`;
+					}
+
 					return create_dynamic_module(
 						'public',
 						vite_config_env.command === 'serve' ? env.public : undefined
@@ -455,31 +464,29 @@ export function set_assets(path) {
 						input[name] = path.resolve(file);
 					});
 				} else {
-					/** @type {Record<string, string>} */
-					input.start = `${runtime_directory}/client/start.js`;
-					input.app = `${kit.outDir}/generated/client-optimized/app.js`;
+					input['entry/start'] = `${runtime_directory}/client/start.js`;
+					input['entry/app'] = `${kit.outDir}/generated/client-optimized/app.js`;
 
-					manifest_data.nodes.forEach((node) => {
-						if (node.component) {
-							const resolved = path.resolve(node.component);
-							const relative = decodeURIComponent(path.relative(kit.files.routes, resolved));
+					/**
+					 * @param {string | undefined} file
+					 */
+					function add_input(file) {
+						if (!file) return;
 
-							const name = relative.startsWith('..')
-								? path.basename(node.component)
-								: posixify(path.join('pages', relative));
-							input[`components/${name}`] = resolved;
-						}
+						const resolved = path.resolve(file);
+						const relative = decodeURIComponent(path.relative(kit.files.routes, resolved));
 
-						if (node.universal) {
-							const resolved = path.resolve(node.universal);
-							const relative = decodeURIComponent(path.relative(kit.files.routes, resolved));
+						const name = relative.startsWith('..')
+							? path.basename(file).replace(/^\+/, '')
+							: relative.replace(/(\\|\/)\+/g, '-').replace(/[\\/]/g, '-');
 
-							const name = relative.startsWith('..')
-								? path.basename(node.universal)
-								: posixify(path.join('pages', relative));
-							input[`modules/${name}`] = resolved;
-						}
-					});
+						input[`entry/${name}`] = resolved;
+					}
+
+					for (const node of manifest_data.nodes) {
+						add_input(node.component);
+						add_input(node.universal);
+					}
 				}
 
 				new_config = {
@@ -491,9 +498,13 @@ export function set_assets(path) {
 							input,
 							output: {
 								format: 'esm',
-								entryFileNames: ssr ? '[name].js' : `${prefix}/[name]-[hash].js`,
-								chunkFileNames: ssr ? 'chunks/[name].js' : `${prefix}/chunks/[name]-[hash].js`,
-								assetFileNames: `${prefix}/assets/[name]-[hash][extname]`,
+								// we use .mjs for client-side modules, because this signals to Chrome (when it
+								// reads the <link rel="preload">) that it should parse the file as a module
+								// rather than as a script, preventing a double parse. Ideally we'd just use
+								// modulepreload, but Safari prevents that
+								entryFileNames: ssr ? '[name].js' : `${prefix}/[name].[hash].mjs`,
+								chunkFileNames: ssr ? 'chunks/[name].js' : `${prefix}/chunks/[name].[hash].mjs`,
+								assetFileNames: `${prefix}/assets/[name].[hash][extname]`,
 								hoistTransitiveImports: false
 							},
 							preserveEntrySignatures: 'strict'

diff --git a/packages/kit/src/runtime/client/start.js b/packages/kit/src/runtime/client/start.js
@@ -1,7 +1,7 @@
 import { DEV } from 'esm-env';
 import { create_client } from './client.js';
 import { init } from './singletons.js';
-import { set_assets, set_version, set_public_env } from '../shared.js';
+import { set_assets, set_version } from '../shared.js';
 
 /**
  * @param {{
@@ -34,5 +34,3 @@ export async function start({ app, assets, hydrate, target, version }) {
 
 	client._start_router();
 }
-
-export { set_public_env as env };
diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js
@@ -141,17 +141,9 @@ export async function render_response({
 		}
 
 		for (const { node } of branch) {
-			if (node.imports) {
-				node.imports.forEach((url) => modulepreloads.add(url));
-			}
-
-			if (node.stylesheets) {
-				node.stylesheets.forEach((url) => stylesheets.add(url));
-			}
-
-			if (node.fonts) {
-				node.fonts.forEach((url) => fonts.add(url));
-			}
+			for (const url of node.imports) modulepreloads.add(url);
+			for (const url of node.stylesheets) stylesheets.add(url);
+			for (const url of node.fonts) fonts.add(url);
 
 			if (node.inline_styles) {
 				Object.entries(await node.inline_styles()).forEach(([k, v]) => inline_styles.set(k, v));
@@ -161,7 +153,8 @@ export async function render_response({
 		rendered = { head: '', html: '', css: { code: '', map: null } };
 	}
 
-	let head = '';
+	let head = `
+		<script>window.__sveltekit_${options.version_hash}={env:${s(public_env)}}</script>`;
 	let body = rendered.html;
 
 	const csp = new Csp(options.csp, {
@@ -318,24 +311,24 @@ export async function render_response({
 
 		// prettier-ignore
 		const init_app = `
-			import { env, start } from ${s(prefixed(client.start.file))};
-
-			env(${s(public_env)});
+			import { start } from ${s(prefixed(client.start.file))};
 
 			start({
 				${opts.join(',\n\t\t\t\t')}
 			});
 		`;
 
-		for (const dep of modulepreloads) {
-			const path = prefixed(dep);
-
-			if (resolve_opts.preload({ type: 'js', path })) {
-				link_header_preloads.add(`<${encodeURI(path)}>; rel="modulepreload"; nopush`);
-				if (state.prerendering) {
-					head += `\n\t\t<link rel="modulepreload" href="${path}">`;
-				}
-			}
+		const included_modulepreloads = Array.from(modulepreloads, (dep) => prefixed(dep)).filter(
+			(path) => resolve_opts.preload({ type: 'js', path })
+		);
+
+		for (const path of included_modulepreloads) {
+			// we use modulepreload with the Link header for Chrome, along with
+			// <link rel="preload"> for Safari. This results in the fastest loading in
+			// the most used browsers, with no double-loading. Note that we need to use
+			// .mjs extensions for `preload` to behave like `modulepreload` in Chrome
+			link_header_preloads.add(`<${encodeURI(path)}>; rel="modulepreload"; nopush`);
+			head += `\n\t\t<link rel="preload" as="script" crossorigin="anonymous" href="${path}">`;
 		}
 
 		const attributes = ['type="module"', `data-sveltekit-hydrate="${target}"`];

diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js
@@ -544,7 +544,7 @@ test.describe('data-sveltekit attributes', () => {
 
 		const module = process.env.DEV
 			? `${baseURL}/src/routes/data-sveltekit/preload-data/target/+page.svelte`
-			: `${baseURL}/_app/immutable/components/pages/data-sveltekit/preload-data/target/_page`;
+			: `${baseURL}/_app/immutable/entry/data-sveltekit-preload-data-target-page`;
 
 		await page.goto('/data-sveltekit/preload-data');
 		await page.locator('#one').dispatchEvent('mousemove');

diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js
@@ -502,7 +502,7 @@ test.describe('Prefetching', () => {
 		} else {
 			// the preload helper causes an additional request to be made in Firefox,
 			// so we use toBeGreaterThan rather than toBe
-			expect(requests.filter((req) => req.endsWith('.js')).length).toBeGreaterThan(0);
+			expect(requests.filter((req) => req.endsWith('.mjs')).length).toBeGreaterThan(0);
 		}
 
 		expect(requests.includes(`${baseURL}/routing/preloading/preloaded.json`)).toBe(true);

diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js
@@ -29,7 +29,7 @@ test.describe('Imports', () => {
 			]);
 		} else {
 			expect(sources[0].startsWith('data:image/png;base64,')).toBeTruthy();
-			expect(sources[1]).toBe(`${baseURL}/_app/immutable/assets/large-3183867c.jpg`);
+			expect(sources[1]).toBe(`${baseURL}/_app/immutable/assets/large.3183867c.jpg`);
 		}
 	});
 });

diff --git a/packages/kit/test/apps/options-2/test/test.js b/packages/kit/test/apps/options-2/test/test.js
@@ -32,7 +32,7 @@ test.describe('Service worker', () => {
 		const response = await request.get('/basepath/service-worker.js');
 		const content = await response.text();
 
-		expect(content).toMatch(/\/_app\/immutable\/start-[a-z0-9]+\.js/);
+		expect(content).toMatch(/\/_app\/immutable\/entry\/start\.[a-z0-9]+\.mjs/);
 	});
 
 	test('does not register /basepath/service-worker.js', async ({ page }) => {

diff --git a/packages/kit/test/apps/options/test/test.js b/packages/kit/test/apps/options/test/test.js
@@ -233,7 +233,7 @@ test.describe('trailingSlash', () => {
 		if (process.env.DEV) {
 			expect(requests.filter((req) => req.endsWith('.svelte')).length).toBe(1);
 		} else {
-			expect(requests.filter((req) => req.endsWith('.js')).length).toBeGreaterThan(0);
+			expect(requests.filter((req) => req.endsWith('.mjs')).length).toBeGreaterThan(0);
 		}
 
 		expect(requests.includes(`/path-base/preloading/preloaded/__data.json`)).toBe(true);
@@ -262,7 +262,7 @@ test.describe('trailingSlash', () => {
 		if (process.env.DEV) {
 			expect(requests.filter((req) => req.endsWith('.svelte')).length).toBe(1);
 		} else {
-			expect(requests.filter((req) => req.endsWith('.js')).length).toBeGreaterThan(0);
+			expect(requests.filter((req) => req.endsWith('.mjs')).length).toBeGreaterThan(0);
 		}
 
 		requests = [];

diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts
@@ -95,6 +95,11 @@ export interface ClientHooks {
 	handleError: HandleClientError;
 }
 
+export interface Env {
+	private: Record<string, string>;
+	public: Record<string, string>;
+}
+
 export class InternalServer extends Server {
 	init(options: ServerInitOptions): Promise<void>;
 	respond(
@@ -323,6 +328,7 @@ export interface SSROptions {
 		}): string;
 		error(values: { message: string; status: number }): string;
 	};
+	version_hash: string;
 }
 
 export interface SSRErrorPage {