force Vite to bundle component libraries in SSR (#1148)

* force Vite to bundle component libraries in SSR - fixes #904 * get things working * lockfile, again * update docs * Update documentation/faq/70-packages.md Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> * remove ssr config from svelte.config.cjs * changeset Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
sveltejs · Apr 22, 2021 · 4645ad5 · 4645ad5 · vercel · Apr 22, 2021
1 parent abf0248
commit 4645ad5
Show file tree

Hide file tree

Showing 9 changed files with 63 additions and 83 deletions.
diff --git a/.changeset/few-points-taste.md b/.changeset/few-points-taste.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+Force Vite to bundle Svelte component libraries in SSR
diff --git a/.changeset/nervous-steaks-add.md b/.changeset/nervous-steaks-add.md
@@ -0,0 +1,5 @@
+---
+'create-svelte': patch
+---
+
+Remove obsolete vite.ssr config from template
diff --git a/documentation/faq/70-packages.md b/documentation/faq/70-packages.md
@@ -2,13 +2,14 @@
 question: How do I fix the error I'm getting trying to include a package?
 ---
 
-Most of these issues come from Vite trying to deal with non-ESM libraries. You may find it helpful to search for your error message in [the Vite issue tracker](https://github.com/vitejs/vite/issues). 
+Most of these issues come from Vite trying to deal with non-ESM libraries. You may find it helpful to search for your error message in [the Vite issue tracker](https://github.com/vitejs/vite/issues).
 
 There are a number of known Vite issues, which cause errors in the following circumstances:
-- [packages which use `exports` instead of `module.exports`](https://github.com/vitejs/vite/issues/2579).
+
+- [CommonJS packages](https://github.com/vitejs/vite/issues/2579).
 - [ESM library that imports a CJS library](https://github.com/vitejs/vite/issues/3024)
 - [some UMD libraries](https://github.com/vitejs/vite/issues/2679)
 
 Vite 2 is a relatively new library and over time we expect it to become easier to use non-ESM libraries with Vite. However, you might also consider asking the library author to distribute an ESM version of their package or even converting the source for the package entirely to ESM. ESM is now the standard way to write JavaScript libraries and while there are a lot of legacy packages out there the ecosystem will become much easier to work with as more libraries convert to ESM.
 
-The most common workarounds would be to try moving the package between `dependencies` and `devDependencies` or trying to `include` or `exclude` it in `optimizeDeps`. SvelteKit currently asks Vite to bundle all your `dependencies` for easier deployment to serverless environment. This means that by moving a dependency to `devDependencies` Vite is no longer asked to bundle it. This may sidestep issues Vite encounters in trying to bundle certain libraries. Avoiding Vite bundling especially works for `adapter-node` and `adapter-static` where the bundling isn't necessary since you're not running in a serverless environment. We are considering [better alternatives](https://github.com/sveltejs/kit/issues/1016) to make this setup easier. You should also add any Svelte components to `ssr.noExternal`. [We hope to do this automatically in the future](https://github.com/sveltejs/kit/issues/904).
+The most common workarounds would be to `include` or `exclude` the offending package in [`optimizeDeps`](https://vitejs.dev/config/#dep-optimization-options), or to add it to [`ssr.external` or `ssr.noExternal`](https://vitejs.dev/config/#ssr-options).
diff --git a/packages/create-svelte/shared/+typescript/svelte.config.cjs b/packages/create-svelte/shared/+typescript/svelte.config.cjs
@@ -15,12 +15,6 @@ module.exports = {
 		adapter: node(),
 
 		// hydrate the <div id="svelte"> element in src/app.html
-		target: '#svelte',
-
-		vite: {
-			ssr: {
-				noExternal: Object.keys(pkg.dependencies || {})
-			}
-		}
+		target: '#svelte'
 	}
 };
diff --git a/packages/create-svelte/shared/-typescript/svelte.config.cjs b/packages/create-svelte/shared/-typescript/svelte.config.cjs
@@ -10,12 +10,6 @@ module.exports = {
 		adapter: node(),
 
 		// hydrate the <div id="svelte"> element in src/app.html
-		target: '#svelte',
-
-		vite: {
-			ssr: {
-				noExternal: Object.keys(pkg.dependencies || {})
-			}
-		}
+		target: '#svelte'
 	}
 };
diff --git a/packages/kit/src/core/build/index.js b/packages/kit/src/core/build/index.js
@@ -2,7 +2,7 @@ import fs from 'fs';
 import path from 'path';
 import { rimraf } from '../filesystem/index.js';
 import create_manifest_data from '../../core/create_manifest_data/index.js';
-import { copy_assets, posixify, resolve_entry } from '../utils.js';
+import { copy_assets, get_no_external, posixify, resolve_entry } from '../utils.js';
 import { create_app } from '../../core/create_app/index.js';
 import vite from 'vite';
 import svelte from '@sveltejs/vite-plugin-svelte';
@@ -438,11 +438,9 @@ async function build_server(
 		// @ts-ignore
 		ssr: {
 			...user_config.ssr,
-			noExternal: [
-				'svelte',
-				'@sveltejs/kit',
-				...((user_config.ssr && user_config.ssr.noExternal) || [])
-			]
+			// note to self: this _might_ need to be ['svelte', '@sveltejs/kit', ...get_no_external()]
+			// but I'm honestly not sure. roll with this for now and see if it's ok
+			noExternal: get_no_external(cwd, user_config.ssr && user_config.ssr.noExternal)
 		},
 		optimizeDeps: {
 			entries: []

diff --git a/packages/kit/src/core/dev/index.js b/packages/kit/src/core/dev/index.js
@@ -11,7 +11,7 @@ import { create_app } from '../../core/create_app/index.js';
 import { rimraf } from '../filesystem/index.js';
 import { ssr } from '../../runtime/server/index.js';
 import { getRawBody } from '../http/index.js';
-import { copy_assets } from '../utils.js';
+import { copy_assets, get_no_external } from '../utils.js';
 import svelte from '@sveltejs/vite-plugin-svelte';
 import { get_server } from '../server/index.js';
 import '../../install-fetch.js';
@@ -107,6 +107,10 @@ class Watcher extends EventEmitter {
 			optimizeDeps: {
 				...user_config.optimizeDeps,
 				entries: []
+			},
+			ssr: {
+				...user_config.ssr,
+				noExternal: get_no_external(this.cwd, user_config.ssr && user_config.ssr.noExternal)
 			}
 		});
 

diff --git a/packages/kit/src/core/utils.js b/packages/kit/src/core/utils.js
@@ -74,3 +74,35 @@ export function resolve_entry(entry) {
 export function posixify(str) {
 	return str.replace(/\\/g, '/');
 }
+
+/**
+ * Get a list of packages that use pkg.svelte, so they can be added
+ * to ssr.noExternal. This is done on a best-effort basis to reduce
+ * the frequency of 'Must use import to load ES Module' and similar
+ * @param {string} cwd
+ * @returns {string[]}
+ */
+function find_svelte_packages(cwd) {
+	const pkg_file = path.join(cwd, 'package.json');
+	if (!fs.existsSync(pkg_file)) return [];
+
+	const pkg = JSON.parse(fs.readFileSync(pkg_file, 'utf8'));
+
+	const deps = [...Object.keys(pkg.dependencies || {}), ...Object.keys(pkg.devDependencies || {})];
+
+	return deps.filter((dep) => {
+		const dep_pkg_file = path.join(cwd, 'node_modules', dep, 'package.json');
+		if (!fs.existsSync(dep_pkg_file)) return false;
+
+		const dep_pkg = JSON.parse(fs.readFileSync(dep_pkg_file, 'utf-8'));
+		return !!dep_pkg.svelte;
+	});
+}
+
+/**
+ * @param {string} cwd
+ * @param {string[]} [user_specified_deps]
+ */
+export function get_no_external(cwd, user_specified_deps = []) {
+	return [...user_specified_deps, ...find_svelte_packages(cwd)];
+}
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml