Skip to content

Commit

Permalink
fix: separate framework code from app code (#8957)
Browse files Browse the repository at this point in the history
* separate app code from framework code

* fix, simplify

* changeset

* DRY

* try this?

* Revert "try this?"

This reverts commit a08b5d8.

* simplify

* debug logs

* Revert "debug logs"

This reverts commit ec618f5.

* bump timeout

* 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

* tidy up

* get rid of set_version

* set assets from window

* load app eagerly

* huh

* fix test

* fix

* tidy up

* changesets

---------

Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
  • Loading branch information
Rich-Harris and dummdidumm authored Feb 19, 2023
1 parent fa18dc5 commit 46eec82
Show file tree
Hide file tree
Showing 40 changed files with 316 additions and 237 deletions.
5 changes: 5 additions & 0 deletions .changeset/brave-eggs-sparkle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: set public env before starting app
5 changes: 5 additions & 0 deletions .changeset/eighty-peas-do.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: preload modules on Safari
5 changes: 5 additions & 0 deletions .changeset/lemon-camels-applaud.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: make `assets` work in client when app is served from a subfolder
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
16 changes: 6 additions & 10 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 @@ -44,12 +39,12 @@ export function create_dynamic_module(type, dev_values) {
);
return `export const env = {\n${keys.join(',\n')}\n}`;
}
return `export { ${type}_env as env } from '${runtime_base}/shared.js';`;
return `export { ${type}_env as env } from '${runtime_base}/shared-server.js';`;
}

/**
* @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
2 changes: 1 addition & 1 deletion packages/kit/src/core/generate_manifest/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export function generate_manifest({ build_data, relative_path, routes }) {
assets: new Set(${s(assets)}),
mimeTypes: ${s(get_mime_lookup(build_data.manifest_data))},
_: {
entry: ${s(build_data.client_entry)},
client: ${s(build_data.client)},
nodes: [
${(node_paths).map(loader).join(',\n\t\t\t\t')}
],
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: 3 additions & 2 deletions packages/kit/src/core/sync/write_client_manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,8 @@ export function write_client_manifest(kit, manifest_data, output, metadata) {

const hooks_file = resolve_entry(kit.files.hooks.client);

// String representation of __CLIENT__/manifest.js
write_if_changed(
`${output}/manifest.js`,
`${output}/app.js`,
trim(`
${hooks_file ? `import * as client_hooks from '${relative_path(output, hooks_file)}';` : ''}
Expand All @@ -125,6 +124,8 @@ export function write_client_manifest(kit, manifest_data, output, metadata) {
hooks_file ? 'client_hooks.handleError || ' : ''
}(({ error }) => { console.error(error) }),
};
export { default as root } from '../root.svelte';
`)
);

Expand Down
10 changes: 6 additions & 4 deletions 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 All @@ -25,11 +26,11 @@ const server_template = ({
error_page
}) => `
import root from '../root.svelte';
import { set_assets, set_building, set_private_env, set_public_env, set_version } from '${runtime_directory}/shared.js';
set_version(${s(config.kit.version.name)});
import { set_building } from '__sveltekit/environment';
import { set_assets, set_private_env, set_public_env } from '${runtime_directory}/shared-server.js';
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},
Expand All @@ -50,7 +51,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
1 change: 1 addition & 0 deletions packages/kit/src/exports/vite/build/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import path from 'node:path';
* @param {import('vite').Manifest} manifest
* @param {string} entry
* @param {boolean} add_dynamic_css
* @returns {import('types').AssetDependencies}
*/
export function find_deps(manifest, entry, add_dynamic_css) {
/** @type {Set<string>} */
Expand Down
24 changes: 15 additions & 9 deletions packages/kit/src/exports/vite/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,19 @@ export async function dev(vite, vite_config, svelte_config) {
assets: new Set(manifest_data.assets.map((asset) => asset.file)),
mimeTypes: get_mime_lookup(manifest_data),
_: {
entry: {
file: `${runtime_base}/client/start.js`,
imports: [],
stylesheets: [],
fonts: []
client: {
start: {
file: `${runtime_base}/client/start.js`,
imports: [],
stylesheets: [],
fonts: []
},
app: {
file: `${svelte_config.kit.outDir}/generated/client/app.js`,
imports: [],
stylesheets: [],
fonts: []
}
},
nodes: manifest_data.nodes.map((node, index) => {
return async () => {
Expand Down Expand Up @@ -443,15 +451,13 @@ export async function dev(vite, vite_config, svelte_config) {
await vite.ssrLoadModule(`${runtime_base}/server/index.js`)
);

const { set_assets, set_version, set_fix_stack_trace } =
const { set_assets, set_fix_stack_trace } =
/** @type {import('types').ServerInternalModule} */ (
await vite.ssrLoadModule(`${runtime_base}/shared.js`)
await vite.ssrLoadModule(`${runtime_base}/shared-server.js`)
);

set_assets(assets);

set_version(svelte_config.kit.version.name);

set_fix_stack_trace(fix_stack_trace);

const server = new Server(manifest);
Expand Down
Loading

0 comments on commit 46eec82

Please sign in to comment.