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

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

Merged
merged 26 commits into from
Feb 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/adapter-static/test/apps/prerendered/.env
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
PUBLIC_ANSWER=42
1 change: 1 addition & 0 deletions packages/adapter-static/test/apps/prerendered/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ node_modules
/.svelte-kit
/build
/functions
!/.env
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
import { env } from '$env/dynamic/public';
</script>

<h1>The answer is {env.PUBLIC_ANSWER}</h1>
5 changes: 5 additions & 0 deletions packages/adapter-static/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
14 changes: 5 additions & 9 deletions packages/kit/src/core/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/

/**
Expand Down Expand Up @@ -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) {
Expand All @@ -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;`);
Expand Down
11 changes: 6 additions & 5 deletions packages/kit/src/core/sync/write_ambient.js
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand All @@ -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)}
`;

/**
Expand All @@ -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)
);
}
5 changes: 4 additions & 1 deletion packages/kit/src/core/sync/write_server.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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%')},
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
csp: ${s(config.kit.csp)},
csrf_check_origin: ${s(config.kit.csrf.checkOrigin)},
embedded: ${config.kit.embedded},
Expand All @@ -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() {
Expand Down
59 changes: 35 additions & 24 deletions packages/kit/src/exports/vite/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;`;
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
}

return create_dynamic_module(
'public',
vite_config_env.command === 'serve' ? env.public : undefined
Expand Down Expand Up @@ -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 = {
Expand All @@ -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]`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity: why change the hyphens to dots?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a minor thing, but between these...

sverdle-how-to-play-page.js-13bb5e39.js
sverdle-how-to-play-page.js.13bb5e39.js

...the second makes more sense to me, because you have four sections of the filename:

[route].[ext].[hash].js

When they're joined by a hyphen, the extension and the hash become one section which doesn't really make sense

hoistTransitiveImports: false
},
preserveEntrySignatures: 'strict'
Expand Down
4 changes: 1 addition & 3 deletions packages/kit/src/runtime/client/start.js
Original file line number Diff line number Diff line change
@@ -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 {{
Expand Down Expand Up @@ -34,5 +34,3 @@ export async function start({ app, assets, hydrate, target, version }) {

client._start_router();
}

export { set_public_env as env };
41 changes: 17 additions & 24 deletions packages/kit/src/runtime/server/page/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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, {
Expand Down Expand Up @@ -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}"`];
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/apps/basics/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
}
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/apps/options-2/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/test/apps/options/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 = [];
Expand Down
6 changes: 6 additions & 0 deletions packages/kit/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -323,6 +328,7 @@ export interface SSROptions {
}): string;
error(values: { message: string; status: number }): string;
};
version_hash: string;
}

export interface SSRErrorPage {
Expand Down