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 Vite's publicDir and base options #5601

Closed
wants to merge 35 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
6e0d0bd
set Vite's publicDir option and change Vite's output dir
benmccann Jul 18, 2022
e1d9529
fixes
benmccann Jul 18, 2022
1eebe2e
fix
benmccann Jul 18, 2022
9600c7b
third time's the charm
benmccann Jul 18, 2022
cc958dd
merge master
benmccann Jul 18, 2022
63724ef
finish merge
benmccann Jul 18, 2022
08edcc6
remove writeStatic
benmccann Jul 18, 2022
3073800
missed a spot
Rich-Harris Jul 18, 2022
8600c38
fix options-2 tests by using Vite's base option
benmccann Jul 18, 2022
da434e9
keep writeStatic, but throw
Rich-Harris Jul 19, 2022
5edd5f3
shruggie
Rich-Harris Jul 19, 2022
e4e9033
update changeset
benmccann Jul 19, 2022
1f65e05
trailing slash + vite base compat
benmccann Jul 19, 2022
9aed215
can't figure this one out. push to CI and see if it gives the same error
benmccann Jul 19, 2022
4521b11
probably don't need this TODO anymore
benmccann Jul 19, 2022
f1e22db
remove outdated test
benmccann Jul 19, 2022
c7bc94e
crawling improvement
benmccann Jul 19, 2022
0c7c581
Revert "crawling improvement"
benmccann Jul 19, 2022
9cbd92c
merge master
benmccann Jul 19, 2022
9ec256e
update changelog
benmccann Jul 19, 2022
f01e437
update changeset
benmccann Jul 19, 2022
ed99a2f
merge master
benmccann Jul 19, 2022
5b62370
cleanup from merging master
benmccann Jul 19, 2022
ded83da
restore TODO
benmccann Jul 19, 2022
ea1bb13
Merge branch 'master' into publicDir
benmccann Jul 19, 2022
c1457a2
format
benmccann Jul 19, 2022
e74f728
fix adapter-static tests
benmccann Jul 19, 2022
a382755
fix unit tests
benmccann Jul 19, 2022
eee66db
fix options test
benmccann Jul 19, 2022
14b7875
format
benmccann Jul 19, 2022
774b70c
prerender updates
benmccann Jul 19, 2022
5ef88c7
restore and update preview message
benmccann Jul 19, 2022
44381e4
restore test
benmccann Jul 20, 2022
2798f9d
Revert "restore test"
benmccann Jul 20, 2022
e1d80c8
move code back to where it was originally
benmccann Jul 20, 2022
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
6 changes: 6 additions & 0 deletions .changeset/four-pandas-push.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sveltejs/kit': patch
'@sveltejs/adapter-node': patch
---

[breaking] set Vite's `publicDir` and `base` options
1 change: 0 additions & 1 deletion packages/adapter-node/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export default function (opts = {}) {
if (precompress) {
builder.log.minor('Compressing assets');
await compress(`${out}/client`);
await compress(`${out}/static`);
await compress(`${out}/prerendered`);
}
}
Expand Down
5 changes: 1 addition & 4 deletions packages/kit/src/core/adapt/builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,7 @@ export function create_builder({ config, build_data, prerendered, log }) {
},

writeClient(dest) {
return [
...copy(`${config.kit.outDir}/output/client`, dest),
...copy(config.kit.files.assets, dest)
];
return [...copy(`${config.kit.outDir}/output/client`, dest)];
},

writePrerendered(dest, { fallback } = {}) {
Expand Down
8 changes: 1 addition & 7 deletions packages/kit/src/core/adapt/builder.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,7 @@ test('copy files', () => {
builder.writeClient(dest);

assert.equal(
[
...glob('**', {
cwd: /** @type {import('types').ValidatedConfig} */ (mocked).kit.files.assets,
dot: true
}),
...glob('**', { cwd: `${outDir}/output/client`, dot: true })
],
glob('**', { cwd: `${outDir}/output/client`, dot: true }),
glob('**', { cwd: dest, dot: true })
);

Expand Down
17 changes: 9 additions & 8 deletions packages/kit/src/core/prerender/prerender.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,24 +103,22 @@ export async function prerender({ config, entries, files, log }) {
/**
* @param {string | null} referrer
* @param {string} decoded
* @param {string} [encoded]
*/
function enqueue(referrer, decoded, encoded) {
function enqueue(referrer, decoded) {
if (seen.has(decoded)) return;
seen.add(decoded);

const file = decoded.slice(config.paths.base.length + 1);
if (files.has(file)) return;

return q.add(() => visit(decoded, encoded || encodeURI(decoded), referrer));
return q.add(() => visit(decoded, referrer));
}

/**
* @param {string} decoded
* @param {string} encoded
* @param {string?} referrer
*/
async function visit(decoded, encoded, referrer) {
async function visit(decoded, referrer) {
if (!decoded.startsWith(config.paths.base)) {
error({ status: 404, path: decoded, referrer, referenceType: 'linked' });
return;
Expand All @@ -129,7 +127,8 @@ export async function prerender({ config, entries, files, log }) {
/** @type {Map<string, import('types').PrerenderDependency>} */
const dependencies = new Map();

const response = await server.respond(new Request(`http://sveltekit-prerender${encoded}`), {
const path = encodeURI(decoded.slice(config.paths.base.length));
const response = await server.respond(new Request(`http://sveltekit-prerender${path}`), {
getClientAddress,
prerendering: {
dependencies
Expand All @@ -138,6 +137,8 @@ export async function prerender({ config, entries, files, log }) {

const body = Buffer.from(await response.arrayBuffer());

const encoded = encodeURI(decoded);

save('pages', response, body, decoded, encoded, referrer, 'linked');

for (const [dependency_path, result] of dependencies) {
Expand Down Expand Up @@ -171,7 +172,7 @@ export async function prerender({ config, entries, files, log }) {
// TODO warn that query strings have no effect on statically-exported pages
}

enqueue(decoded, decodeURI(pathname), pathname);
enqueue(decoded, decodeURI(pathname));
}
}
}
Expand Down Expand Up @@ -201,7 +202,7 @@ export async function prerender({ config, entries, files, log }) {
if (location) {
const resolved = resolve(encoded, location);
if (is_root_relative(resolved)) {
enqueue(decoded, decodeURI(resolved), resolved);
enqueue(decoded, decodeURI(resolved));
}

if (!response.headers.get('x-sveltekit-normalize')) {
Expand Down
10 changes: 2 additions & 8 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,6 @@ export async function respond(request, options, state) {
/** @type {Record<string, string>} */
let params = {};

if (options.paths.base && !state.prerendering?.fallback) {
if (!decoded.startsWith(options.paths.base)) {
return new Response('Not found', { status: 404 });
}
decoded = decoded.slice(options.paths.base.length) || '/';
}

const is_data_request = decoded.endsWith(DATA_SUFFIX);

if (is_data_request) {
Expand Down Expand Up @@ -93,13 +86,14 @@ export async function respond(request, options, state) {
const normalized = normalize_path(url.pathname, options.trailing_slash);

if (normalized !== url.pathname && !state.prerendering?.fallback) {
const path = options.paths.base + normalized;
return new Response(undefined, {
status: 301,
headers: {
'x-sveltekit-normalize': '1',
location:
// ensure paths starting with '//' are not treated as protocol-relative
(normalized.startsWith('//') ? url.origin + normalized : normalized) +
(normalized.startsWith('//') ? url.origin + path : path) +
(url.search === '?' ? '' : url.search)
}
});
Expand Down
4 changes: 1 addition & 3 deletions packages/kit/src/vite/build/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,7 @@ export const get_default_config = function ({ config, input, ssr, outDir }) {
__SVELTEKIT_APP_VERSION_FILE__: JSON.stringify(`${config.kit.appDir}/version.json`),
__SVELTEKIT_APP_VERSION_POLL_INTERVAL__: JSON.stringify(config.kit.version.pollInterval)
},
// prevent Vite copying the contents of `config.kit.files.assets`,
// if it happens to be 'public' instead of 'static'
publicDir: false,
publicDir: ssr ? false : config.kit.files.assets,
resolve: {
alias: get_aliases(config.kit)
},
Expand Down
55 changes: 0 additions & 55 deletions packages/kit/src/vite/dev/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import fs from 'fs';
import colors from 'kleur';
import path from 'path';
import sirv from 'sirv';
import { URL } from 'url';
import { getRequest, setResponse } from '../../node/index.js';
import { installPolyfills } from '../../node/polyfills.js';
Expand Down Expand Up @@ -173,12 +172,6 @@ export async function dev(vite, vite_config, svelte_config) {
}

const assets = svelte_config.kit.paths.assets ? SVELTE_KIT_ASSETS : svelte_config.kit.paths.base;
const asset_server = sirv(svelte_config.kit.files.assets, {
dev: true,
etag: true,
maxAge: 0,
extensions: []
});

return () => {
const serve_static_middleware = vite.middlewares.stack.find(
Expand All @@ -197,20 +190,6 @@ export async function dev(vite, vite_config, svelte_config) {
}`;

const decoded = decodeURI(new URL(base + req.url).pathname);

if (decoded.startsWith(assets)) {
const pathname = decoded.slice(assets.length);
const file = svelte_config.kit.files.assets + pathname;

if (fs.existsSync(file) && !fs.statSync(file).isDirectory()) {
if (has_correct_case(file, svelte_config.kit.files.assets)) {
req.url = encodeURI(pathname); // don't need query/hash
asset_server(req, res);
return;
}
}
}

const file = posixify(path.resolve(decoded.slice(1)));
const is_file = fs.existsSync(file) && !fs.statSync(file).isDirectory();
const allowed =
Expand All @@ -223,13 +202,6 @@ export async function dev(vite, vite_config, svelte_config) {
return;
}

if (!decoded.startsWith(svelte_config.kit.paths.base)) {
return not_found(
res,
`Not found (did you mean ${svelte_config.kit.paths.base + req.url}?)`
);
}

/** @type {Partial<import('types').Hooks>} */
const user_hooks = resolve_entry(svelte_config.kit.files.hooks)
? await vite.ssrLoadModule(`/${svelte_config.kit.files.hooks}`)
Expand Down Expand Up @@ -383,12 +355,6 @@ export async function dev(vite, vite_config, svelte_config) {
};
}

/** @param {import('http').ServerResponse} res */
function not_found(res, message = 'Not found') {
res.statusCode = 404;
res.end(message);
}

/**
* @param {import('connect').Server} server
*/
Expand Down Expand Up @@ -444,24 +410,3 @@ async function find_deps(vite, node, deps) {

await Promise.all(branches);
}

/**
* Determine if a file is being requested with the correct case,
* to ensure consistent behaviour between dev and prod and across
* operating systems. Note that we can't use realpath here,
* because we don't want to follow symlinks
* @param {string} file
* @param {string} assets
* @returns {boolean}
*/
function has_correct_case(file, assets) {
if (file === assets) return true;

const parent = path.dirname(file);

if (fs.readdirSync(parent).includes(path.basename(file))) {
return has_correct_case(parent, assets);
}

return false;
}
3 changes: 2 additions & 1 deletion packages/kit/src/vite/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ function kit() {
/** @type {import('vite').UserConfig} */
const result = {
appType: 'custom',
base: '/',
base: `${svelte_config.kit.paths.base}/`,
build: {
rollupOptions: {
// Vite dependency crawler needs an explicit JS entry point
Expand All @@ -208,6 +208,7 @@ function kit() {
define: {
__SVELTEKIT_APP_VERSION_POLL_INTERVAL__: '0'
},
publicDir: svelte_config.kit.files.assets,
resolve: {
alias: get_aliases(svelte_config.kit)
},
Expand Down
51 changes: 27 additions & 24 deletions packages/kit/src/vite/preview/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ export async function preview(vite, config, protocol) {
const server = new Server(manifest);

return () => {
// files in `static`
vite.middlewares.use(scoped(assets, mutable(config.kit.files.assets)));

// immutable generated client assets
vite.middlewares.use(
scoped(
Expand All @@ -70,7 +67,11 @@ export async function preview(vite, config, protocol) {
next();
} else {
res.statusCode = 404;
res.end(`Not found (did you mean ${base + pathname}?)`);
res.end(
`The server is configured with a public base URL of ${base}/ - did you mean to visit <a href="${
base + pathname
}">${base + pathname}</a> instead?`
);
}
});

Expand Down Expand Up @@ -122,29 +123,31 @@ export async function preview(vite, config, protocol) {
);

// SSR
vite.middlewares.use(async (req, res) => {
const host = req.headers['host'];
vite.middlewares.use(
scoped(base, async (req, res) => {
const host = req.headers['host'];

let request;
let request;

try {
request = await getRequest(`${protocol}://${host}`, req);
} catch (/** @type {any} */ err) {
res.statusCode = err.status || 400;
return res.end(err.reason || 'Invalid request body');
}
try {
request = await getRequest(`${protocol}://${host}`, req);
} catch (/** @type {any} */ err) {
res.statusCode = err.status || 400;
return res.end(err.reason || 'Invalid request body');
}

setResponse(
res,
await server.respond(request, {
getClientAddress: () => {
const { remoteAddress } = req.socket;
if (remoteAddress) return remoteAddress;
throw new Error('Could not determine clientAddress');
}
})
);
});
setResponse(
res,
await server.respond(request, {
getClientAddress: () => {
const { remoteAddress } = req.socket;
if (remoteAddress) return remoteAddress;
throw new Error('Could not determine clientAddress');
}
})
);
})
);
};
}

Expand Down
5 changes: 0 additions & 5 deletions packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -473,11 +473,6 @@ test.describe('Static files', () => {
expect(await r2.json()).toEqual({ works: true });
});

test('Filenames are case-sensitive', async ({ request }) => {
const response = await request.get('/static.JSON');
expect(response.status()).toBe(404);
});
Comment on lines -476 to -479
Copy link
Member

Choose a reason for hiding this comment

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

why was this test deleted?

Copy link
Member Author

@benmccann benmccann Jul 20, 2022

Choose a reason for hiding this comment

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

Vite does not check case sensitivity

Copy link
Member

Choose a reason for hiding this comment

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

Right, but many servers do, so it's essential that we do too — if we can't get Vite to be strict about casing it's a showstopper for this PR


test('Serves symlinked asset', async ({ request }) => {
const response = await request.get('/symlink-from/hello.txt');
expect(response.status()).toBe(200);
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/test/apps/options-2/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ test.describe.configure({ mode: 'parallel' });

test.describe('paths.base', () => {
test('serves /basepath', async ({ page }) => {
await page.goto('/basepath');
await page.goto('/basepath/');
Copy link
Member

Choose a reason for hiding this comment

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

what's the reason for this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Vite only serves the index page with a trailing slash

Copy link
Member Author

Choose a reason for hiding this comment

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

Related issue: vitejs/vite#8770

Copy link
Member

Choose a reason for hiding this comment

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

okay, but the path is incorrect

expect(await page.textContent('h1')).toBe('Hello');
});

Expand All @@ -28,7 +28,7 @@ test.describe('Service worker', () => {
});

test('does not register /basepath/service-worker.js', async ({ page }) => {
await page.goto('/basepath');
await page.goto('/basepath/');
expect(await page.content()).not.toMatch(/navigator\.serviceWorker/);
});
});
6 changes: 4 additions & 2 deletions packages/kit/test/apps/options/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ test.describe.configure({ mode: 'parallel' });

test.describe('base path', () => {
test('serves a useful 404 when visiting unprefixed path', async ({ request }) => {
const response = await request.get('/');
const response = await request.get('/slash/', { headers: { Accept: 'text/html' } });
expect(response.status()).toBe(404);
expect(await response.text()).toBe('Not found (did you mean /path-base/?)');
expect(await response.text()).toBe(
'The server is configured with a public base URL of /path-base/ - did you mean to visit <a href="/path-base/slash/">/path-base/slash/</a> instead?'
);
});

test('serves /', async ({ page, javaScriptEnabled }) => {
Expand Down