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

dev/preview paths #2189

Merged
merged 20 commits into from
Aug 13, 2021
Merged
Show file tree
Hide file tree
Changes from 12 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
37 changes: 20 additions & 17 deletions packages/kit/src/core/build/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,11 @@ export async function build(config, { cwd = process.cwd(), runtime = '@sveltejs/
cwd,
config,
build_dir,
base:
config.kit.paths.assets === '/.'
? `/${config.kit.appDir}/`
: `${config.kit.paths.assets}/${config.kit.appDir}/`,
// TODO this is so that Vite's preloading works. Unfortunately, it fails
// during `svelte-kit preview`, because we use a local asset path. If Vite
// used relative paths, I _think_ this could get fixed. Issue here:
// https://github.com/vitejs/vite/issues/2009
base: `${config.kit.paths.assets || config.kit.paths.base}/${config.kit.appDir}/`,
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
manifest: create_manifest_data({
config,
output: build_dir,
Expand All @@ -58,9 +59,7 @@ export async function build(config, { cwd = process.cwd(), runtime = '@sveltejs/
await build_server(options, client_manifest, runtime);

if (options.service_worker_entry_file) {
const { base, assets } = config.kit.paths;

if (assets !== base && assets !== '/.') {
if (config.kit.paths.assets) {
throw new Error('Cannot use service worker alongside config.kit.paths.assets');
}

Expand Down Expand Up @@ -239,7 +238,7 @@ async function build_server(
return relative_file[0] === '.' ? relative_file : `./${relative_file}`;
};

const prefix = `${config.kit.paths.assets}/${config.kit.appDir}/`;
const prefix = `/${config.kit.appDir}/`;

/**
* @param {string} file
Expand Down Expand Up @@ -270,8 +269,8 @@ async function build_server(

find_deps(file, js_deps, css_deps);

const js = Array.from(js_deps).map((url) => prefix + url);
const css = Array.from(css_deps).map((url) => prefix + url);
const js = Array.from(js_deps);
const css = Array.from(css_deps);

const styles = config.kit.amp
? Array.from(css_deps).map((url) => {
Expand All @@ -281,7 +280,7 @@ async function build_server(
: [];

metadata_lookup[file] = {
entry: prefix + client_manifest[file].file,
entry: client_manifest[file].file,
css,
js,
styles
Expand All @@ -301,7 +300,7 @@ async function build_server(
`
import { respond } from '${runtime}';
import root from './generated/root.svelte';
import { set_paths } from './runtime/paths.js';
import { set_paths, assets } from './runtime/paths.js';
import { set_prerendering } from './runtime/env.js';
import * as user_hooks from ${s(app_relative(hooks_file))};

Expand All @@ -323,13 +322,13 @@ async function build_server(
amp: ${config.kit.amp},
dev: false,
entry: {
file: ${s(prefix + client_manifest[client_entry_file].file)},
css: ${s(Array.from(entry_css).map(dep => prefix + dep))},
js: ${s(Array.from(entry_js).map(dep => prefix + dep))}
file: assets + ${s(prefix + client_manifest[client_entry_file].file)},
benmccann marked this conversation as resolved.
Show resolved Hide resolved
css: [${Array.from(entry_css).map(dep => 'assets + ' + s(prefix + dep))}],
js: [${Array.from(entry_js).map(dep => 'assets + ' + s(prefix + dep))}]
},
fetched: undefined,
floc: ${config.kit.floc},
get_component_path: id => ${s(`${config.kit.paths.assets}/${config.kit.appDir}/`)} + entry_lookup[id],
get_component_path: id => assets + ${s(prefix)} + entry_lookup[id],
get_stack: error => String(error), // for security
handle_error: /** @param {Error & {frame?: string}} error */ (error) => {
if (error.frame) {
Expand Down Expand Up @@ -407,9 +406,13 @@ async function build_server(
const metadata_lookup = ${s(metadata_lookup)};

async function load_component(file) {
const { entry, css, js, styles } = metadata_lookup[file];
return {
module: await module_lookup[file](),
...metadata_lookup[file]
entry: assets + ${s(prefix)} + entry,
css: css.map(dep => assets + ${s(prefix)} + dep),
js: js.map(dep => assets + ${s(prefix)} + dep),
styles
};
}

Expand Down
40 changes: 14 additions & 26 deletions packages/kit/src/core/config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,30 +68,6 @@ function validate(definition, option, keypath) {
return merged;
}

/**
* @param {string} from
* @param {string} to
*/
function resolve(from, to) {
// the `/.` is weird, but allows `${assets}/images/blah.jpg` to work
// when `assets` is empty
return remove_trailing_slash(url.resolve(add_trailing_slash(from), to)) || '/.';
}

/**
* @param {string} str
*/
function add_trailing_slash(str) {
return str.endsWith('/') ? str : `${str}/`;
}

/**
* @param {string} str
*/
function remove_trailing_slash(str) {
return str.endsWith('/') ? str.slice(0, -1) : str;
}

/**
* @param {string} cwd
* @param {import('types/config').ValidatedConfig} validated
Expand Down Expand Up @@ -153,14 +129,26 @@ export function validate_config(config) {
);
}

if (paths.assets) {
if (!/^[a-z]+:\/\//.test(paths.assets)) {
throw new Error(
'kit.paths.assets option must be an absolute path, if specified. See https://kit.svelte.dev/docs#configuration-paths'
);
}

if (paths.assets.endsWith('/')) {
throw new Error(
"kit.paths.assets option must not end with '/'. See https://kit.svelte.dev/docs#configuration-paths"
);
}
}

if (appDir.startsWith('/') || appDir.endsWith('/')) {
throw new Error(
"kit.appDir cannot start or end with '/'. See https://kit.svelte.dev/docs#configuration"
);
}

paths.assets = resolve(paths.base, paths.assets);

return validated;
}

Expand Down
76 changes: 27 additions & 49 deletions packages/kit/src/core/config/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ test('fills in defaults', () => {
},
paths: {
base: '',
assets: '/.'
assets: ''
},
prerender: {
crawl: true,
Expand Down Expand Up @@ -147,7 +147,7 @@ test('fills in partial blanks', () => {
},
paths: {
base: '',
assets: '/.'
assets: ''
},
prerender: {
crawl: true,
Expand Down Expand Up @@ -230,6 +230,30 @@ test("fails if paths.base ends with '/'", () => {
}, /^kit\.paths\.base option must be a root-relative path that starts but doesn't end with '\/'. See https:\/\/kit\.svelte\.dev\/docs#configuration-paths$/);
});

test('fails if paths.assets is relative', () => {
assert.throws(() => {
validate_config({
kit: {
paths: {
assets: 'foo'
}
}
});
}, /^kit\.paths\.assets option must be an absolute path, if specified. See https:\/\/kit\.svelte\.dev\/docs#configuration-paths$/);
});

test('fails if paths.assets has trailing slash', () => {
assert.throws(() => {
validate_config({
kit: {
paths: {
assets: 'https://cdn.example.com/stuff/'
}
}
});
}, /^kit\.paths\.assets option must not end with '\/'. See https:\/\/kit\.svelte\.dev\/docs#configuration-paths$/);
});

test('fails if prerender.pages are invalid', () => {
assert.throws(() => {
validate_config({
Expand Down Expand Up @@ -260,60 +284,14 @@ function validate_paths(name, input, output) {
});
}

validate_paths(
'assets relative to empty string',
{
assets: 'path/to/assets'
},
{
base: '',
assets: '/path/to/assets'
}
);

validate_paths(
'assets relative to base path',
{
base: '/path/to/base',
assets: 'path/to/assets'
},
{
base: '/path/to/base',
assets: '/path/to/base/path/to/assets'
}
);

validate_paths(
'empty assets relative to base path',
{
base: '/path/to/base'
},
{
base: '/path/to/base',
assets: '/path/to/base'
}
);

validate_paths(
'root-relative assets',
{
assets: '/path/to/assets'
},
{
base: '',
assets: '/path/to/assets'
}
);

validate_paths(
'root-relative assets with base path',
{
base: '/path/to/base',
assets: '/path/to/assets'
},
{
base: '/path/to/base',
assets: '/path/to/assets'
assets: ''
}
);

Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/core/config/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ async function testLoadDefaultConfig(path) {
serviceWorker: {
exclude: []
},
paths: { base: '', assets: '/.' },
paths: { base: '', assets: '' },
prerender: { crawl: true, enabled: true, force: undefined, onError: 'fail', pages: ['*'] },
router: true,
ssr: true,
Expand Down
5 changes: 5 additions & 0 deletions packages/kit/src/core/constants.js
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
export const SVELTE_KIT = '.svelte-kit';

// in `svelte-kit dev` and `svelte-kit preview`, we use a fake
// asset path so that we can serve local assets while still
// verifying that requests are correctly prefixed
export const SVELTE_KIT_ASSETS = '/_svelte_kit_assets';
17 changes: 14 additions & 3 deletions packages/kit/src/core/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { deep_merge, print_config_conflicts } from '../config/index.js';
import { svelte } from '@sveltejs/vite-plugin-svelte';
import { get_server } from '../server/index.js';
import { __fetch_polyfill } from '../../install-fetch.js';
import { SVELTE_KIT } from '../constants.js';
import { SVELTE_KIT, SVELTE_KIT_ASSETS } from '../constants.js';

/** @typedef {{ cwd?: string, port: number, host?: string, https: boolean, config: import('types/config').ValidatedConfig }} Options */
/** @typedef {import('types/internal').SSRComponent} SSRComponent */
Expand Down Expand Up @@ -169,7 +169,8 @@ class Watcher extends EventEmitter {
ssr: {
// @ts-expect-error ssr is considered in alpha, so not yet exposed by Vite
noExternal: [...((vite_config.ssr && vite_config.ssr.noExternal) || []), ...svelte_packages]
}
},
base: this.config.kit.paths.assets.startsWith('/') ? `${this.config.kit.paths.assets}/` : '/'
});

print_config_conflicts(conflicts, 'kit.vite.');
Expand Down Expand Up @@ -313,6 +314,13 @@ async function create_handler(vite, config, dir, cwd, manifest) {

const root = (await vite.ssrLoadModule(`/${dir}/generated/root.svelte`)).default;

const paths = await vite.ssrLoadModule(`/${SVELTE_KIT}/dev/runtime/paths.js`);

paths.set_paths({
base: config.kit.paths.base,
assets: config.kit.paths.assets ? SVELTE_KIT_ASSETS : config.kit.paths.base
});

let body;

try {
Expand Down Expand Up @@ -363,7 +371,10 @@ async function create_handler(vite, config, dir, cwd, manifest) {
serverFetch: hooks.serverFetch || fetch
},
hydrate: config.kit.hydrate,
paths: config.kit.paths,
paths: {
base: config.kit.paths.base,
assets: config.kit.paths.assets ? SVELTE_KIT_ASSETS : config.kit.paths.base
},
load_component: async (id) => {
const url = path.resolve(cwd, id);

Expand Down
Loading