feat: enable prebundling by default (#494)

* feat: enable prebundleSvelteLibraries by default

* docs: add information about switching from deep imports to package imports for prebundled

* docs: update prebundling faq

* docs: update changeset to remove outdated advice

* docs: Apply suggestions from code review

Co-authored-by: Ben McCann <[email protected]>

* docs: improve recommendations

* fix: make sure prebundleSvelteLibraries is not used for build or ssr

* docs: Apply suggestions from code review

Co-authored-by: Ben McCann <[email protected]>

* docs: Apply suggestions from code review

Co-authored-by: Bjorn Lu <[email protected]>

* docs: update follow-up wording after previous changes

* fix: only enable prebundleSvelteLibraries for dev, log warnings if it is enabled during build or ssr

* docs: update default value in docs

* fix: remove warning for ssr.optimizeDeps, we don't add the svelte esbuild plugin for that so it does whatever, but won't break because of prebundleSvelteLibraries

* Apply suggestions from code review

Co-authored-by: Bjorn Lu <[email protected]>

Co-authored-by: Ben McCann <[email protected]>
Co-authored-by: Bjorn Lu <[email protected]>

Author: 3 people

.changeset/calm-rules-push.md

@@ -0,0 +1,7 @@
+---
+'@sveltejs/vite-plugin-svelte': minor
+---
+
+enable `prebundleSvelteLibraries` during dev by default to improve page loading for the dev server.
+
+see the [FAQ](https://github.com/sveltejs/vite-plugin-svelte/blob/main/docs/faq.md#what-is-going-on-with-vite-and-pre-bundling-dependencies) for more information about `prebundleSvelteLibraries` and how to tune it.

docs/config.md

@@ -204,9 +204,13 @@ A [picomatch pattern](https://github.com/micromatch/picomatch), or array of patt
 ### prebundleSvelteLibraries
 
 - **Type:** `boolean`
-- **Default:** `false`
+- **Default:** `true` for dev, `false` for build
+
+  Enable [Vite's dependency prebundling](https://vitejs.dev/guide/dep-pre-bundling.html) for Svelte libraries.
+
+  This option improves page loading for the dev server in most applications when using Svelte component libraries.
 
-  Force Vite to pre-bundle Svelte libraries. Setting this `true` should improve initial page load performance, especially when using large Svelte libraries. See the [FAQ](./faq.md#what-is-going-on-with-vite-and-pre-bundling-dependencies) for details of the pre-bundling implementation.
+  See the [FAQ](./faq.md#what-is-going-on-with-vite-and-pre-bundling-dependencies) for details and how to fine-tune it for huge libraries.
 
 ## Experimental options
 

docs/faq.md

@@ -99,4 +99,81 @@ For reference, check out [windicss](https://github.com/windicss/vite-plugin-wind
 
 ### What is going on with Vite and `Pre-bundling dependencies:`?
 
-Pre-bundling dependencies is an [optimization in Vite](https://vitejs.dev/guide/dep-pre-bundling.html). It is required for CJS dependencies, as Vite's development server only works with ES modules on the client side. Importantly for Svelte libraries and ESM modules, prebundling combines component libraries into a single file to speed up the initial page load. Try setting the [`prebundleSvelteLibraries`](./config.md#prebundleSvelteLibraries) option to `true` to speed things up. This will likely be enabled by default in future version of the plugin.
+Prebundling dependencies is an [optimization in Vite](https://vitejs.dev/guide/dep-pre-bundling.html).
+
+> We only use prebundling during **development**, the following does not apply to or describe the built application
+
+It is required for CJS dependencies, as Vite's development server only works with ES modules on the client side.
+Importantly for Svelte libraries and ES modules, it also reduces the number of http requests when you load a page from the dev server and caches files so subsequent starts are even faster.
+
+The way prebundling Svelte libraries affects your dev-server load times depends on the import style you use, index or deep:
+
+#### Index imports
+
+Offers better DX but can cause noticable delays on your machine, especially for libraries with many files.
+
+```diff
+import { SomeComponent } from 'some-library'
++ only one request per library
++ intellisense for the whole library after first import
+- compiles the whole library even if you only use a few components
+- slower build and dev-server ssr
+```
+
+#### Deep imports
+
+Offers snappier dev and faster builds for libraries with many files at the expense of some DX
+
+```diff
+import SomeComponent from 'some-library/src/SomeComponent.svelte'
++ compiles only the components you import
++ faster build and dev-server ssr
+- one request per import can slow down initial load if you use a lot of components
+- intellisense only for imported components
+```
+
+#### Rewriting imports with plugins or preprocessors
+
+**Do not use it in combination with prebundling!**
+
+Prebundling works by reading your `.svelte` files from disk and scanning them for imports. It cannot detect
+added/changed/removed imports and these then cause extra requests, delays and render the prebundled files from the initial scan moot.
+If you prefer to use these tools, please exclude the libraries you use them with from prebundling.
+
+#### Excluding libraries from prebundling
+
+If you want to disable prebundling for a single library, use `optimizeDeps.exclude`
+
+```js
+// vite.config.js
+export default defineConfig({
+  optimizeDeps: {
+    exclude: ['some-library'] // do not pre-bundle some-library
+  }
+});
+```
+
+Or disable it for all Svelte libraries
+
+```js
+// svelte.config.js
+export default {
+  vitePlugin: {
+    prebundleSvelteLibraries: false
+  }
+};
+```
+
+#### Recommendations
+
+There is no golden rule, but you can follow these recommendations:
+
+1. **Never** combine plugins or preprocessors that rewrite imports with prebundling
+2. Start with index imports and if your dev-server or build process feels slow, check compile stats to see if switching to deep imports can improve the experience.
+3. Do not mix deep and index imports for the same library, use one style consistently.
+4. Use different import styles for different libraries where it helps. E.g. deep imports for the few icons of that one huge icon library, but index import for the component library that is heavily used.
+
+#### I get a warning `Incompatible options: prebundleSvelteLibraries ...`
+
+This warning only occurs if you use non-default settings in your vite config that can cause problems in combination with prebundleSvelteLibraries.
+You should not use prebundleSvelteLibraries during build or for ssr, disable one of the incompatible options to make that warning (and subsequent errors) go away.

packages/e2e-tests/package-json-svelte-field/__tests__/package-json-svelte-field.spec.ts

@@ -1,18 +1,5 @@
-import { getText, isBuild, readVitePrebundleMetadata } from '~utils';
+import { getText } from '~utils';
 
 test('should render component imported via svelte field in package.json', async () => {
 	expect(await getText('#test-id')).toBe('svelte field works');
 });
-
-if (!isBuild) {
-	test('should optimize nested cjs deps of excluded svelte deps', () => {
-		const metadataFile = readVitePrebundleMetadata();
-		const metadata = JSON.parse(metadataFile);
-		const optimizedPaths = Object.keys(metadata.optimized);
-		expect(optimizedPaths).not.toContain('e2e-test-dep-svelte-nested');
-		expect(optimizedPaths).not.toContain('e2e-test-dep-svelte-simple');
-		expect(optimizedPaths).toContain(
-			'e2e-test-dep-svelte-nested > e2e-test-dep-svelte-simple > e2e-test-dep-cjs-only'
-		);
-	});
-}

packages/e2e-tests/package-json-svelte-field/vite.config.js

@@ -4,6 +4,9 @@ import { defineConfig } from 'vite';
 export default defineConfig(({ command, mode }) => {
 	return {
 		plugins: [svelte()],
+		optimizeDeps: {
+			exclude: ['e2e-test-dep-scss-only']
+		},
 		build: {
 			// make build faster by skipping transforms and minification
 			target: 'esnext',

packages/e2e-tests/prebundle-svelte-deps/__tests__/prebundle-svelte-deps.spec.ts

@@ -1,33 +1,28 @@
-import { browserLogs, getText, isBuild, readVitePrebundleMetadata } from '~utils';
+import {
+	browserLogs,
+	editFile,
+	getText,
+	isBuild,
+	readVitePrebundleMetadata,
+	waitForServerRestartAndReloadPage
+} from '~utils';
 
-test('should not have failed requests', async () => {
+async function expectPageToWork() {
 	browserLogs.forEach((msg) => {
 		expect(msg).not.toMatch('404');
 	});
-});
-
-test('should render Hybrid import', async () => {
 	expect(await getText('#hybrid .label')).toBe('dependency-import');
-});
-
-test('should render Simple import', async () => {
-	expect(await getText('#simple .label')).toBe('dependency-import');
-});
-
-test('should render Exports Simple import', async () => {
-	expect(await getText('#exports-simple .label')).toBe('dependency-import');
-});
-
-test('should render Nested import', async () => {
 	expect(await getText('#nested #message')).toBe('nested');
 	expect(await getText('#nested #cjs-and-esm')).toBe('esm');
-});
-
-test('should render api-only import', async () => {
 	expect(await getText('#api-only')).toBe('api loaded: true');
-});
+	expect(await getText('#simple .label')).toBe('dependency-import');
+	expect(await getText('#exports-simple .label')).toBe('dependency-import');
+}
 
 if (!isBuild) {
+	test('page works with pre-bundling enabled', async () => {
+		await expectPageToWork();
+	});
 	test('should optimize svelte dependencies', () => {
 		const metadataFile = readVitePrebundleMetadata();
 		const metadata = JSON.parse(metadataFile);
@@ -46,4 +41,30 @@ if (!isBuild) {
 		expect(optimizedPaths).not.toContain('e2e-test-dep-svelte-hybrid');
 		expect(optimizedPaths).toContain('e2e-test-dep-svelte-hybrid > e2e-test-dep-cjs-only');
 	});
+
+	test('page works with pre-bundling disabled', async () => {
+		editFile('svelte.config.js', (c) =>
+			c.replace('prebundleSvelteLibraries: true', 'prebundleSvelteLibraries: false')
+		);
+		await waitForServerRestartAndReloadPage();
+		await expectPageToWork();
+		const metadataFile = readVitePrebundleMetadata();
+		const metadata = JSON.parse(metadataFile);
+		const optimizedPaths = Object.keys(metadata.optimized);
+		expect(optimizedPaths).not.toContain('e2e-test-dep-svelte-simple');
+		expect(optimizedPaths).not.toContain('e2e-test-dep-svelte-hybrid');
+
+		// this is a bit surprising, we always include js-libraries using svelte
+		expect(optimizedPaths).toContain('e2e-test-dep-svelte-api-only');
+
+		expect(optimizedPaths).toContain('e2e-test-dep-svelte-hybrid > e2e-test-dep-cjs-only');
+		expect(optimizedPaths).toContain('e2e-test-dep-svelte-simple > e2e-test-dep-cjs-only');
+		expect(optimizedPaths).toContain(
+			'e2e-test-dep-svelte-nested > e2e-test-dep-svelte-simple > e2e-test-dep-cjs-only'
+		);
+	});
+} else {
+	test('page works', async () => {
+		await expectPageToWork();
+	});
 }

packages/e2e-tests/prebundle-svelte-deps/vite.config.js

@@ -16,5 +16,13 @@ export default defineConfig({
 		// make build faster by skipping transforms and minification
 		target: 'esnext',
 		minify: false
+	},
+	server: {
+		watch: {
+			// During tests we edit the files too fast and sometimes chokidar
+			// misses change events, so enforce polling for consistency
+			usePolling: true,
+			interval: 100
+		}
 	}
 });

packages/vite-plugin-svelte/src/utils/options.ts

@@ -135,17 +135,19 @@ export async function preResolveOptions(
 		...viteUserConfig,
 		root: resolveViteRoot(viteUserConfig)
 	};
+	const isBuild = viteEnv.command === 'build';
 	const defaultOptions: Partial<Options> = {
 		extensions: ['.svelte'],
-		emitCss: true
+		emitCss: true,
+		prebundleSvelteLibraries: !isBuild
 	};
 	const svelteConfig = convertPluginOptions(
 		await loadSvelteConfig(viteConfigWithResolvedRoot, inlineOptions)
 	);
 
 	const extraOptions: Partial<PreResolvedOptions> = {
 		root: viteConfigWithResolvedRoot.root!,
-		isBuild: viteEnv.command === 'build',
+		isBuild,
 		isServe: viteEnv.command === 'serve',
 		isDebug: process.env.DEBUG != null
 	};
@@ -373,12 +375,17 @@ export async function buildExtraViteConfig(
 
 	// handle prebundling for svelte files
 	if (options.prebundleSvelteLibraries) {
-		extraViteConfig.optimizeDeps.extensions = options.extensions ?? ['.svelte'];
-		// Add esbuild plugin to prebundle Svelte files.
-		// Currently a placeholder as more information is needed after Vite config is resolved,
-		// the real Svelte plugin is added in `patchResolvedViteConfig()`
-		extraViteConfig.optimizeDeps.esbuildOptions = {
-			plugins: [{ name: facadeEsbuildSveltePluginName, setup: () => {} }]
+		extraViteConfig.optimizeDeps = {
+			...extraViteConfig.optimizeDeps,
+			// Experimental Vite API to allow these extensions to be scanned and prebundled
+			// @ts-ignore
+			extensions: options.extensions ?? ['.svelte'],
+			// Add esbuild plugin to prebundle Svelte files.
+			// Currently a placeholder as more information is needed after Vite config is resolved,
+			// the real Svelte plugin is added in `patchResolvedViteConfig()`
+			esbuildOptions: {
+				plugins: [{ name: facadeEsbuildSveltePluginName, setup: () => {} }]
+			}
 		};
 	}
 
@@ -392,9 +399,44 @@ export async function buildExtraViteConfig(
 		log.debug('enabling "experimental.hmrPartialAccept" in vite config');
 		extraViteConfig.experimental = { hmrPartialAccept: true };
 	}
+	validateViteConfig(extraViteConfig, config, options);
 	return extraViteConfig;
 }
 
+function validateViteConfig(
+	extraViteConfig: Partial<UserConfig>,
+	config: UserConfig,
+	options: PreResolvedOptions
+) {
+	const { prebundleSvelteLibraries, isBuild } = options;
+	if (prebundleSvelteLibraries) {
+		const isEnabled = (option: 'dev' | 'build' | boolean) =>
+			option !== true && option !== (isBuild ? 'build' : 'dev');
+		const logWarning = (name: string, value: 'dev' | 'build' | boolean, recommendation: string) =>
+			log.warn.once(
+				`Incompatible options: \`prebundleSvelteLibraries: true\` and vite \`${name}: ${JSON.stringify(
+					value
+				)}\` ${isBuild ? 'during build.' : '.'} ${recommendation}`
+			);
+		const viteOptimizeDepsDisabled = config.optimizeDeps?.disabled ?? 'build'; // fall back to vite default
+		const isOptimizeDepsEnabled = isEnabled(viteOptimizeDepsDisabled);
+		if (!isBuild && !isOptimizeDepsEnabled) {
+			logWarning(
+				'optimizeDeps.disabled',
+				viteOptimizeDepsDisabled,
+				'Forcing `optimizeDeps.disabled: "build"`. Disable prebundleSvelteLibraries or update your vite config to enable optimizeDeps during dev.'
+			);
+			extraViteConfig.optimizeDeps!.disabled = 'build';
+		} else if (isBuild && isOptimizeDepsEnabled) {
+			logWarning(
+				'optimizeDeps.disabled',
+				viteOptimizeDepsDisabled,
+				'Disable optimizeDeps or prebundleSvelteLibraries for build if you experience errors.'
+			);
+		}
+	}
+}
+
 async function buildExtraConfigForDependencies(options: PreResolvedOptions, config: UserConfig) {
 	// extra handling for svelte dependencies in the project
 	const depsConfig = await crawlFrameworkPkgs({
@@ -576,9 +618,11 @@ export interface PluginOptions {
 	disableDependencyReinclusion?: boolean | string[];
 
 	/**
-	 * Force Vite to pre-bundle Svelte libraries
+	 * Enable support for Vite's dependency optimization to prebundle Svelte libraries.
 	 *
-	 * @default false
+	 * To disable prebundling for a specific library, add it to `optimizeDeps.exclude`.
+	 *
+	 * @default true for dev, false for build
 	 */
 	prebundleSvelteLibraries?: boolean;