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

feat: set dynamic base when rendering page #9220

Merged
merged 23 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/few-lions-drive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': minor
---

feat: add `paths.relative` option to control interpretation of `paths.assets` and `paths.base`
2 changes: 1 addition & 1 deletion packages/adapter-static/test/test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import fs from 'fs';
import fs from 'node:fs';
import * as assert from 'uvu/assert';
import { run } from './utils.js';

Expand Down
18 changes: 12 additions & 6 deletions packages/kit/src/core/config/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ const get_defaults = (prefix = '') => ({
typescript: {},
paths: {
base: '',
assets: ''
assets: '',
relative: undefined
},
prerender: {
concurrency: 1,
Expand Down Expand Up @@ -235,6 +236,7 @@ test('fails if paths.base is not root-relative', () => {
validate_config({
kit: {
paths: {
// @ts-expect-error
base: 'https://example.com/somewhere/else'
}
}
Expand All @@ -259,6 +261,7 @@ test('fails if paths.assets is relative', () => {
validate_config({
kit: {
paths: {
// @ts-expect-error
assets: 'foo'
}
}
Expand Down Expand Up @@ -293,8 +296,8 @@ test('fails if prerender.entries are invalid', () => {

/**
* @param {string} name
* @param {{ base?: string, assets?: string }} input
* @param {{ base?: string, assets?: string }} output
* @param {import('types').KitConfig['paths']} input
* @param {import('types').KitConfig['paths']} output
*/
function validate_paths(name, input, output) {
test(name, () => {
Expand All @@ -316,7 +319,8 @@ validate_paths(
},
{
base: '/path/to/base',
assets: ''
assets: '',
relative: undefined
}
);

Expand All @@ -327,7 +331,8 @@ validate_paths(
},
{
base: '',
assets: 'https://cdn.example.com'
assets: 'https://cdn.example.com',
relative: undefined
}
);

Expand All @@ -339,7 +344,8 @@ validate_paths(
},
{
base: '/path/to/base',
assets: 'https://cdn.example.com'
assets: 'https://cdn.example.com',
relative: undefined
}
);

Expand Down
7 changes: 7 additions & 0 deletions packages/kit/src/core/config/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,13 @@ const options = object(
}
}

return input;
}),
relative: validate(undefined, (input, keypath) => {
if (typeof input !== 'boolean') {
throw new Error(`${keypath} option must be a boolean or undefined`);
}

return input;
})
}),
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/core/sync/write_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ const server_template = ({
}) => `
import root from '../root.svelte';
import { set_building } from '__sveltekit/environment';
import { set_assets, set_private_env, set_public_env } from '${runtime_directory}/shared-server.js';
import { set_assets } from '__sveltekit/paths';
import { set_private_env, set_public_env } from '${runtime_directory}/shared-server.js';

export const options = {
app_template_contains_nonce: ${template.includes('%sveltekit.nonce%')},
Expand Down
11 changes: 5 additions & 6 deletions packages/kit/src/exports/vite/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -451,15 +451,14 @@ export async function dev(vite, vite_config, svelte_config) {
await vite.ssrLoadModule(`${runtime_base}/server/index.js`)
);

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

const { set_assets } = await vite.ssrLoadModule('__sveltekit/paths');
set_assets(assets);

set_fix_stack_trace(fix_stack_trace);

const server = new Server(manifest);

await server.init({ env });
Expand Down
25 changes: 21 additions & 4 deletions packages/kit/src/exports/vite/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -374,17 +374,34 @@ function kit({ svelte_config }) {
case '\0__sveltekit/paths':
const { assets, base } = svelte_config.kit.paths;

// use the values defined in `global`, but fall back to hard-coded values
// for the sake of things like Vitest which may import this module
// outside the context of a page
if (browser) {
return `export const base = ${s(base)};
export const assets = ${global}.assets;`;
return `export const base = ${global}?.base ?? ${s(base)};
export const assets = ${global}?.assets ?? ${assets ? s(assets) : 'base'};`;
}

return `export const base = ${s(base)};
return `export let base = ${s(base)};
export let assets = ${assets ? s(assets) : 'base'};

export const relative = ${svelte_config.kit.paths.relative};

const initial = { base, assets };

export function override(paths) {
base = paths.base;
assets = paths.assets;
}

export function reset() {
base = initial.base;
assets = initial.assets;
}

/** @param {string} path */
export function set_assets(path) {
assets = path;
assets = initial.assets = path;
}`;

case '\0__sveltekit/environment':
Expand Down
7 changes: 5 additions & 2 deletions packages/kit/src/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ declare module '__sveltekit/environment' {

/** Internal version of $app/paths */
declare module '__sveltekit/paths' {
export const base: `/${string}`;
export let assets: `https://${string}` | `http://${string}`;
export let base: '' | `/${string}`;
export let assets: '' | `https://${string}` | `http://${string}` | '/_svelte_kit_assets';
export let relative: boolean | undefined; // TODO in 2.0, make this a `boolean` that defaults to `true`
export function reset(): void;
export function override(paths: { base: string; assets: string }): void;
export function set_assets(path: string): void;
}
91 changes: 55 additions & 36 deletions packages/kit/src/runtime/server/page/render.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as devalue from 'devalue';
import { readable, writable } from 'svelte/store';
import { DEV } from 'esm-env';
import { assets, base } from '__sveltekit/paths';
import * as paths from '__sveltekit/paths';
import { hash } from '../../hash.js';
import { serialize_data } from './serialize_data.js';
import { s } from '../../../utils/misc.js';
Expand All @@ -11,6 +11,7 @@ import { clarify_devalue_error, stringify_uses, handle_error_and_jsonify } from
import { public_env } from '../../shared-server.js';
import { text } from '../../../exports/index.js';
import { create_async_iterator } from '../../../utils/streaming.js';
import { SVELTE_KIT_ASSETS } from '../../../constants.js';

// TODO rename this function/module

Expand Down Expand Up @@ -80,6 +81,43 @@ export async function render_response({
? action_result.data ?? null
: null;

/** @type {string} */
let base = paths.base;

/** @type {string} */
let assets = paths.assets;

/**
* An expression that will evaluate in the client to determine the resolved base path.
* We use a relative path when possible to support IPFS, the internet archive, etc.
*/
let base_expression = s(paths.base);

// if appropriate, use relative paths for greater portability
if (paths.relative !== false && !state.prerendering?.fallback) {
const segments = event.url.pathname.slice(paths.base.length).split('/');

if (segments.length === 1 && paths.base !== '') {
// if we're on `/my-base-path`, relative links need to start `./my-base-path` rather than `.`
base = `./${paths.base.split('/').at(-1)}`;
Copy link
Member

Choose a reason for hiding this comment

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

Why split.at(-1)? What if the base path spans more than one /, like /foo/bar? We know the base has to start with a slash, so why not

Suggested change
base = `./${paths.base.split('/').at(-1)}`;
base = `.${paths.base}`;

Copy link
Member Author

Choose a reason for hiding this comment

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

Consider a basepath like /a/b/c. If you're rendering that route, then {base}/d should resolve to /a/b/c/d.

With non-relative paths, that's easy — just use base as written. But if you're using a relative path, then it needs to be ./c to produce ./c/d to resolve to /a/b/c/d (or ../b/c/d, or ../../a/b/c/d, but there's no point in doing that).

With your suggestion, it would be ./a/b/c, which would resolve to /a/b/a/b/c/d


base_expression = `new URL(${s(base)}, location).pathname`;
} else {
base =
segments
.slice(2)
.map(() => '..')
.join('/') || '.';

// resolve e.g. '../..' against current location, then remove trailing slash
base_expression = `new URL(${s(base)}, location).pathname.slice(0, -1)`;
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
}

if (!paths.assets || (paths.assets[0] === '/' && paths.assets !== SVELTE_KIT_ASSETS)) {
assets = base;
}
}

if (page_config.ssr) {
if (__SVELTEKIT_DEV__ && !branch.at(-1)?.node.component) {
// Can only be the leaf, layouts have a fallback component generated
Expand Down Expand Up @@ -116,6 +154,10 @@ export async function render_response({
form: form_value
};

// use relative paths during rendering, so that the resulting HTML is as
// portable as possible, but reset afterwards
if (paths.relative) paths.override({ base, assets });

if (__SVELTEKIT_DEV__) {
const fetch = globalThis.fetch;
let warned = false;
Expand All @@ -138,9 +180,14 @@ export async function render_response({
rendered = options.root.render(props);
} finally {
globalThis.fetch = fetch;
paths.reset();
}
} else {
rendered = options.root.render(props);
try {
rendered = options.root.render(props);
} finally {
paths.reset();
}
}

for (const { node } of branch) {
Expand All @@ -156,35 +203,6 @@ export async function render_response({
rendered = { head: '', html: '', css: { code: '', map: null } };
}

/**
* The prefix to use for static assets. Replaces `%sveltekit.assets%` in the template
* @type {string}
*/
let resolved_assets;

/**
* An expression that will evaluate in the client to determine the resolved asset path
*/
let asset_expression;

if (assets) {
// if an asset path is specified, use it
resolved_assets = assets;
asset_expression = s(assets);
} else if (state.prerendering?.fallback) {
// if we're creating a fallback page, asset paths need to be root-relative
resolved_assets = base;
asset_expression = s(base);
} else {
// otherwise we want asset paths to be relative to the page, so that they
// will work in odd contexts like IPFS, the internet archive, and so on
const segments = event.url.pathname.slice(base.length).split('/').slice(2);
resolved_assets = segments.length > 0 ? segments.map(() => '..').join('/') : '.';
asset_expression = `new URL(${s(
resolved_assets
)}, location.href).pathname.replace(/^\\\/$/, '')`;
}

let head = '';
let body = rendered.html;

Expand All @@ -198,9 +216,9 @@ export async function render_response({
// Vite makes the start script available through the base path and without it.
// We load it via the base path in order to support remote IDE environments which proxy
// all URLs under the base path during development.
return base + path;
return paths.base + path;
}
return `${resolved_assets}/${path}`;
return `${assets}/${path}`;
};

if (inline_styles.size > 0) {
Expand Down Expand Up @@ -285,9 +303,10 @@ export async function render_response({

const properties = [
`env: ${s(public_env)}`,
`assets: ${asset_expression}`,
paths.assets && `assets: ${s(paths.assets)}`,
`base: ${base_expression}`,
`element: document.currentScript.parentElement`
];
].filter(Boolean);

if (chunks) {
blocks.push(`const deferred = new Map();`);
Expand Down Expand Up @@ -418,7 +437,7 @@ export async function render_response({
const html = options.templates.app({
head,
body,
assets: resolved_assets,
assets,
nonce: /** @type {string} */ (csp.nonce),
env: public_env
});
Expand Down
2 changes: 0 additions & 2 deletions packages/kit/src/runtime/shared-server.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
export { set_assets } from '__sveltekit/paths';

/** @type {Record<string, string>} */
export let private_env = {};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
import { base, assets } from '$app/paths';
</script>

<pre>{JSON.stringify({ base, assets })}</pre>
9 changes: 9 additions & 0 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,15 @@ test.describe('$app/paths', () => {
assets: ''
})
);

await page.goto('/paths/deeply/nested');

expect(await page.innerHTML('pre')).toBe(
JSON.stringify({
base: '',
assets: ''
})
);
});

// some browsers will re-request assets after a `pushState`
Expand Down
7 changes: 7 additions & 0 deletions packages/kit/test/apps/options-2/src/routes/+page.svelte
Original file line number Diff line number Diff line change
@@ -1 +1,8 @@
<script>
import { base, assets } from '$app/paths';
</script>

<h1>Hello</h1>

<p data-testid="base">base: {base}</p>
<p data-testid="assets">assets: {assets}</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import { base, assets } from '$app/paths';
</script>

<h1>Hello</h1>

<p data-testid="base">base: {base}</p>
<p data-testid="assets">assets: {assets}</p>
3 changes: 2 additions & 1 deletion packages/kit/test/apps/options-2/svelte.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
const config = {
kit: {
paths: {
base: '/basepath'
base: '/basepath',
relative: true
},
serviceWorker: {
register: false
Expand Down
Loading