Set Vite's publicDir and correctly serve public assets earlier in pip…

…eline (#5686) * Set Vite's publicDir and correctly serve public assets earlier in pipeline * format * get tests passing * format * update test app to prevent regressions * remove incomplete request checks
sveltejs · Jul 25, 2022 · 3d1d2c0 · 3d1d2c0
1 parent 12bdc22
commit 3d1d2c0
Show file tree

Hide file tree

Showing 11 changed files with 55 additions and 46 deletions.
diff --git a/.changeset/light-vans-suffer.md b/.changeset/light-vans-suffer.md
@@ -0,0 +1,6 @@
+---
+'@sveltejs/adapter-node': patch
+'@sveltejs/kit': patch
+---
+
+Set Vite's publicDir and correctly serve public assets earlier in pipeline
diff --git a/packages/adapter-node/index.js b/packages/adapter-node/index.js
@@ -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`);
 			}
 		}

diff --git a/packages/kit/src/core/adapt/builder.js b/packages/kit/src/core/adapt/builder.js
@@ -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 } = {}) {

diff --git a/packages/kit/src/core/adapt/builder.spec.js b/packages/kit/src/core/adapt/builder.spec.js
@@ -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 })
 	);
 

diff --git a/packages/kit/src/core/prerender/prerender.js b/packages/kit/src/core/prerender/prerender.js
@@ -1,4 +1,4 @@
-import { existsSync, readFileSync, writeFileSync } from 'fs';
+import { readFileSync, writeFileSync } from 'fs';
 import { dirname, join } from 'path';
 import { pathToFileURL, URL } from 'url';
 import { mkdirp, posixify, walk } from '../../utils/filesystem.js';
@@ -158,10 +158,7 @@ export async function prerender({ config, client_out_dir, manifest_path, log })
 		return file;
 	}
 
-	const files = new Set([
-		...walk(client_out_dir).map(posixify),
-		...(existsSync(config.files.assets) ? walk(config.files.assets).map(posixify) : []) // TODO remove this if we use Vite publicDir option
-	]);
+	const files = new Set(walk(client_out_dir).map(posixify));
 	const seen = new Set();
 	const written = new Set();
 

diff --git a/packages/kit/src/vite/build/utils.js b/packages/kit/src/vite/build/utils.js
@@ -117,9 +117,7 @@ export const get_default_config = function ({ config, input, ssr, outDir }) {
 			__SVELTEKIT_APP_VERSION_POLL_INTERVAL__: JSON.stringify(config.kit.version.pollInterval),
 			__SVELTEKIT_DEV__: 'false'
 		},
-		// 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)
 		},

diff --git a/packages/kit/src/vite/dev/index.js b/packages/kit/src/vite/dev/index.js
@@ -180,37 +180,50 @@ export async function dev(vite, vite_config, svelte_config) {
 		extensions: []
 	});
 
+	vite.middlewares.use(async (req, res, next) => {
+		try {
+			const base = `${vite.config.server.https ? 'https' : 'http'}://${
+				req.headers[':authority'] || req.headers.host
+			}`;
+
+			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;
+					}
+				}
+			}
+
+			next();
+		} catch (e) {
+			const error = coalesce_to_error(e);
+			res.statusCode = 500;
+			res.end(fix_stack_trace(error));
+		}
+	});
+
 	return () => {
 		const serve_static_middleware = vite.middlewares.stack.find(
 			(middleware) =>
 				/** @type {function} */ (middleware.handle).name === 'viteServeStaticMiddleware'
 		);
 
-		remove_html_middlewares(vite.middlewares);
+		remove_static_middlewares(vite.middlewares);
 
 		vite.middlewares.use(async (req, res) => {
 			try {
-				if (!req.url || !req.method) throw new Error('Incomplete request');
-
 				const base = `${vite.config.server.https ? 'https' : 'http'}://${
 					req.headers[':authority'] || req.headers.host
 				}`;
 
 				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 =
@@ -389,11 +402,15 @@ function not_found(res, message = 'Not found') {
 /**
  * @param {import('connect').Server} server
  */
-function remove_html_middlewares(server) {
-	const html_middlewares = ['viteServeStaticMiddleware'];
+function remove_static_middlewares(server) {
+	// We don't use viteServePublicMiddleware because of the following issues:
+	// /~https://github.com/vitejs/vite/issues/9260
+	// /~https://github.com/vitejs/vite/issues/9236
+	// /~https://github.com/vitejs/vite/issues/9234
+	const static_middlewares = ['viteServePublicMiddleware', 'viteServeStaticMiddleware'];
 	for (let i = server.stack.length - 1; i > 0; i--) {
-		// @ts-expect-error using internals until /~https://github.com/vitejs/vite/pull/4640 is merged
-		if (html_middlewares.includes(server.stack[i].handle.name)) {
+		// @ts-expect-error using internals
+		if (static_middlewares.includes(server.stack[i].handle.name)) {
 			server.stack.splice(i, 1);
 		}
 	}

diff --git a/packages/kit/src/vite/index.js b/packages/kit/src/vite/index.js
@@ -213,6 +213,7 @@ function kit() {
 					__SVELTEKIT_DEV__: 'true',
 					__SVELTEKIT_APP_VERSION_POLL_INTERVAL__: '0'
 				},
+				publicDir: svelte_config.kit.files.assets,
 				resolve: {
 					alias: get_aliases(svelte_config.kit)
 				},

diff --git a/packages/kit/src/vite/preview/index.js b/packages/kit/src/vite/preview/index.js
@@ -44,16 +44,13 @@ 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
+		// generated client assets and the contents of `static`
 		vite.middlewares.use(
 			scoped(
 				assets,
 				sirv(join(config.kit.outDir, 'output/client'), {
 					setHeaders: (res, pathname) => {
-						// only apply to build directory, not e.g. version.json
+						// only apply to immutable directory, not e.g. version.json
 						if (pathname.startsWith(`/${config.kit.appDir}/immutable`)) {
 							res.setHeader('cache-control', 'public,max-age=31536000,immutable');
 						}

diff --git a/packages/kit/test/apps/basics/src/app.html b/packages/kit/test/apps/basics/src/app.html
@@ -5,6 +5,9 @@
 		<meta name="viewport" content="width=device-width, initial-scale=1" />
 		<link rel="icon" type="image/png" href="%sveltekit.assets%/favicon.png" />
 		<meta name="transform-page" content="__REPLACEME__" />
+		<!-- included to ensure Vite tranform middleware isn't run on static scripts
+			   /~https://github.com/sveltejs/kit/issues/5672 -->
+		<script src="%sveltekit.assets%/empty.js"></script>
 		%sveltekit.head%
 	</head>
 	<body>

diff --git a/packages/kit/test/apps/basics/static/empty.js b/packages/kit/test/apps/basics/static/empty.js