fix: only optimize nested cjs dependencies (#163)

* wip: fix nested optimization

* feat: better heuristic

* refactor(options): reuse createRequire

* feat: add test

* chore: add changeset

* fix: update tests

* chore: add github issue link

* perf: skip optimizeDeps config in build

Author: bluwy

.changeset/tricky-clouds-grab.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/vite-plugin-svelte': patch
+---
+
+Only optimize nested cjs dependencies

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

@@ -7,14 +7,13 @@ test('should render component imported via svelte field in package.json', async
 });
 
 if (!isBuild) {
-	test('should optimize nested deps of excluded svelte deps', () => {
+	test('should optimize nested cjs deps of excluded svelte deps', () => {
 		const vitePrebundleMetadata = path.resolve(__dirname, '../node_modules/.vite/_metadata.json');
 		const metadataFile = fs.readFileSync(vitePrebundleMetadata, 'utf8');
 		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-cjs-and-esm');
 		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

@@ -3,11 +3,7 @@ const { defineConfig } = require('vite');
 
 module.exports = defineConfig(({ command, mode }) => {
 	return {
-		plugins: [
-			svelte({
-				disableDependencyReinclusion: ['e2e-test-dep-svelte-hybrid']
-			})
-		],
+		plugins: [svelte()],
 		build: {
 			// make build faster by skipping transforms and minification
 			target: 'esnext',

packages/vite-plugin-svelte/src/index.ts

@@ -52,7 +52,7 @@ export function svelte(inlineOptions?: Partial<Options>): Plugin {
 			}
 			options = await resolveOptions(inlineOptions, config, configEnv);
 			// extra vite config
-			const extraViteConfig = buildExtraViteConfig(options, config);
+			const extraViteConfig = buildExtraViteConfig(options, config, configEnv);
 			log.debug('additional vite config', extraViteConfig);
 			return extraViteConfig as Partial<UserConfig>;
 		},

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

@@ -39,8 +39,8 @@ function getSvelteDependencies(
 	const result = [];
 	const localRequire = createRequire(`${pkgDir}/package.json`);
 	const resolvedDeps = deps
-		.map((dep) => resolveSvelteDependency(dep, localRequire))
-		.filter(Boolean);
+		.map((dep) => resolveDependencyData(dep, localRequire))
+		.filter((depData) => !!depData && isSvelte(depData.pkg)) as DependencyData[];
 	// @ts-ignore
 	for (const { pkg, dir } of resolvedDeps) {
 		result.push({ name: pkg.name, pkg, dir, path });
@@ -64,16 +64,10 @@ function getSvelteDependencies(
 	return result;
 }
 
-function resolveSvelteDependency(
-	dep: string,
-	localRequire: NodeRequire
-): { dir: string; pkg: Pkg } | void {
+function resolveDependencyData(dep: string, localRequire: NodeRequire): DependencyData | void {
 	try {
 		const pkgJson = `${dep}/package.json`;
 		const pkg = localRequire(pkgJson);
-		if (!isSvelte(pkg)) {
-			return;
-		}
 		const dir = path.dirname(localRequire.resolve(pkgJson));
 		return { dir, pkg };
 	} catch (e) {
@@ -84,12 +78,6 @@ function resolveSvelteDependency(
 			while (dir) {
 				const pkg = parsePkg(dir, true);
 				if (pkg && pkg.name === dep) {
-					if (!isSvelte(pkg)) {
-						return;
-					}
-					log.warn.once(
-						`package.json of ${dep} has a "svelte" field but does not include itself in exports field. Please ask package maintainer to update`
-					);
 					return { dir, pkg };
 				}
 				const parent = path.dirname(dir);
@@ -174,6 +162,20 @@ function is_common_without_svelte_field(dependency: string): boolean {
 	);
 }
 
+export function needsOptimization(dep: string, localRequire: NodeRequire): boolean {
+	const depData = resolveDependencyData(dep, localRequire);
+	if (!depData) return false;
+	const pkg = depData.pkg;
+	// only optimize if is cjs, using the below as heuristic
+	// see https://github.com/sveltejs/vite-plugin-svelte/issues/162
+	return pkg.main && !pkg.module && !pkg.exports;
+}
+
+interface DependencyData {
+	dir: string;
+	pkg: Pkg;
+}
+
 export interface SvelteDependency {
 	name: string;
 	dir: string;

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

@@ -13,8 +13,9 @@ import {
 	// eslint-disable-next-line node/no-missing-import
 } from 'svelte/types/compiler/preprocess';
 import path from 'path';
-import { findRootSvelteDependencies, SvelteDependency } from './dependencies';
+import { findRootSvelteDependencies, needsOptimization, SvelteDependency } from './dependencies';
 import { DepOptimizationOptions } from 'vite/src/node/optimizer/index';
+import { createRequire } from 'module';
 
 const knownOptions = new Set([
 	'configFile',
@@ -180,12 +181,12 @@ function resolveViteRoot(viteConfig: UserConfig): string | undefined {
 
 export function buildExtraViteConfig(
 	options: ResolvedOptions,
-	config: UserConfig
+	config: UserConfig,
+	configEnv: ConfigEnv
 ): Partial<UserConfig> {
 	// extra handling for svelte dependencies in the project
 	const svelteDeps = findRootSvelteDependencies(options.root);
 	const extraViteConfig: Partial<UserConfig> = {
-		optimizeDeps: buildOptimizeDepsForSvelte(svelteDeps, options, config.optimizeDeps),
 		resolve: {
 			mainFields: [...SVELTE_RESOLVE_MAIN_FIELDS],
 			dedupe: [...SVELTE_IMPORTS, ...SVELTE_HMR_IMPORTS]
@@ -196,6 +197,14 @@ export function buildExtraViteConfig(
 		// knownJsSrcExtensions: options.extensions
 	};
 
+	if (configEnv.command === 'serve') {
+		extraViteConfig.optimizeDeps = buildOptimizeDepsForSvelte(
+			svelteDeps,
+			options,
+			config.optimizeDeps
+		);
+	}
+
 	// @ts-ignore
 	extraViteConfig.ssr = buildSSROptionsForSvelte(svelteDeps, options, config);
 
@@ -253,11 +262,12 @@ function buildOptimizeDepsForSvelte(
 		}
 		const transitiveDepsToInclude = svelteDeps
 			.filter((dep) => !disabledReinclusions.includes(dep.name) && isExcluded(dep.name))
-			.flatMap((dep) =>
-				Object.keys(dep.pkg.dependencies || {})
-					.filter((depOfDep) => !isExcluded(depOfDep))
-					.map((depOfDep) => dep.path.concat(dep.name, depOfDep).join(' > '))
-			);
+			.flatMap((dep) => {
+				const localRequire = createRequire(`${dep.dir}/package.json`);
+				return Object.keys(dep.pkg.dependencies || {})
+					.filter((depOfDep) => !isExcluded(depOfDep) && needsOptimization(depOfDep, localRequire))
+					.map((depOfDep) => dep.path.concat(dep.name, depOfDep).join(' > '));
+			});
 		log.debug(
 			`reincluding transitive dependencies of excluded svelte dependencies`,
 			transitiveDepsToInclude