fix: don't resolve via svelte field for prebundled libraries (#482)

dominikg · bluwy · web-flow · commit 9dd08b1974a3 · 2022-11-11T23:30:43.000+08:00
Co-authored-by: bluwy &lt;bjornlu.dev@gmail.com&gt;
diff --git a/.changeset/smart-poets-learn.md b/.changeset/smart-poets-learn.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/vite-plugin-svelte': patch
+---
+
+fix(prebundleSvelteLibraries): avoid resolving via svelte field after a library has been prebundled
diff --git a/packages/e2e-tests/_test_dependencies/svelte-hybrid/package.json b/packages/e2e-tests/_test_dependencies/svelte-hybrid/package.json
@@ -12,6 +12,7 @@
   ],
   "dependencies": {
     "@types/node": "^18.11.9",
-    "e2e-test-dep-cjs-only": "workspace:*"
-  }
+    "e2e-test-dep-cjs-only": "file:../cjs-only"
+  },
+  "type": "module"
 }
diff --git a/packages/e2e-tests/_test_dependencies/svelte-nested/package.json b/packages/e2e-tests/_test_dependencies/svelte-nested/package.json
@@ -12,8 +12,8 @@
     "./package.json": "./package.json"
   },
   "dependencies": {
-    "e2e-test-dep-svelte-simple": "workspace:*",
-    "e2e-test-dep-cjs-and-esm": "workspace:*",
-    "e2e-test-dep-scss-only": "workspace:*"
+    "e2e-test-dep-svelte-simple": "file:../svelte-simple",
+    "e2e-test-dep-cjs-and-esm": "file:../cjs-and-esm",
+    "e2e-test-dep-scss-only": "file:../scss-only"
   }
 }
diff --git a/packages/e2e-tests/_test_dependencies/svelte-simple/package.json b/packages/e2e-tests/_test_dependencies/svelte-simple/package.json
@@ -5,6 +5,7 @@
   "main": "index.js",
   "svelte": "index.js",
   "dependencies": {
-    "e2e-test-dep-cjs-only": "workspace:*"
-  }
+    "e2e-test-dep-cjs-only": "file:../cjs-only"
+  },
+  "type": "module"
 }
diff --git a/packages/e2e-tests/autoprefixer-browerslist/package.json b/packages/e2e-tests/autoprefixer-browerslist/package.json
@@ -8,7 +8,7 @@
     "preview": "vite preview"
   },
   "dependencies": {
-    "e2e-test-dep-svelte-simple": "workspace:*"
+    "e2e-test-dep-svelte-simple": "file:../_test_dependencies/svelte-simple"
   },
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "workspace:*",
diff --git a/packages/e2e-tests/configfile-custom/package.json b/packages/e2e-tests/configfile-custom/package.json
@@ -8,7 +8,7 @@
     "preview": "vite preview"
   },
   "dependencies": {
-    "e2e-test-dep-svelte-simple": "workspace:*"
+    "e2e-test-dep-svelte-simple": "file:../_test_dependencies/svelte-simple"
   },
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "workspace:*",
diff --git a/packages/e2e-tests/configfile-esm/package.json b/packages/e2e-tests/configfile-esm/package.json
@@ -8,7 +8,7 @@
     "preview": "vite preview"
   },
   "dependencies": {
-    "e2e-test-dep-svelte-simple": "workspace:*"
+    "e2e-test-dep-svelte-simple": "file:../_test_dependencies/svelte-simple"
   },
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "workspace:*",
diff --git a/packages/e2e-tests/dependencies/package.json b/packages/e2e-tests/dependencies/package.json
@@ -4,10 +4,10 @@
   "private": true,
   "version": "0.0.0",
   "dependencies": {
-    "e2e-test-dep-cjs-and-esm": "workspace:*",
-    "e2e-test-dep-cjs-only": "workspace:*",
-    "e2e-test-dep-esm-only": "workspace:*",
-    "e2e-test-dep-index-only": "workspace:*",
-    "e2e-test-dep-scss-only": "workspace:*"
+    "e2e-test-dep-cjs-and-esm": "file:../_test_dependencies/cjs-and-esm",
+    "e2e-test-dep-cjs-only": "file:../_test_dependencies/cjs-only",
+    "e2e-test-dep-esm-only": "file:../_test_dependencies/esm-only",
+    "e2e-test-dep-index-only": "file:../_test_dependencies/index-only",
+    "e2e-test-dep-scss-only": "file:../_test_dependencies/scss-only"
   }
 }
diff --git a/packages/e2e-tests/hmr/package.json b/packages/e2e-tests/hmr/package.json
@@ -8,11 +8,11 @@
     "preview": "vite preview"
   },
   "dependencies": {
-    "e2e-test-dep-svelte-simple": "workspace:*"
+    "e2e-test-dep-svelte-simple": "file:../_test_dependencies/svelte-simple"
   },
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "workspace:*",
-    "e2e-test-dep-vite-plugins": "workspace:*",
+    "e2e-test-dep-vite-plugins": "file:../_test_dependencies/vite-plugins",
     "node-fetch": "^3.2.10",
     "svelte": "^3.52.0",
     "vite": "^3.2.3"
diff --git a/packages/e2e-tests/package-json-svelte-field/__tests__/package-json-svelte-field.spec.ts b/packages/e2e-tests/package-json-svelte-field/__tests__/package-json-svelte-field.spec.ts
@@ -1,6 +1,4 @@
-import fs from 'fs';
-import path from 'path';
-import { getText, isBuild } from '~utils';
+import { getText, isBuild, readVitePrebundleMetadata } from '~utils';
 
 test('should render component imported via svelte field in package.json', async () => {
 	expect(await getText('#test-id')).toBe('svelte field works');
@@ -18,20 +16,3 @@ if (!isBuild) {
 		);
 	});
 }
-
-function readVitePrebundleMetadata() {
-	const metadataPaths = [
-		'../node_modules/.vite/_metadata.json',
-		'../node_modules/.vite/deps/_metadata.json' // vite 2.9
-	];
-	for (const metadataPath of metadataPaths) {
-		try {
-			const vitePrebundleMetadata = path.resolve(__dirname, metadataPath);
-			const metadataFile = fs.readFileSync(vitePrebundleMetadata, 'utf8');
-			return metadataFile;
-		} catch {
-			// ignore
-		}
-	}
-	throw new Error('Unable to find vite prebundle metadata');
-}
diff --git a/packages/e2e-tests/package-json-svelte-field/package.json b/packages/e2e-tests/package-json-svelte-field/package.json
@@ -8,8 +8,8 @@
     "preview": "vite preview"
   },
   "dependencies": {
-    "e2e-test-dep-svelte-hybrid": "workspace:*",
-    "e2e-test-dep-svelte-nested": "workspace:*"
+    "e2e-test-dep-svelte-hybrid": "file:../_test_dependencies/svelte-hybrid",
+    "e2e-test-dep-svelte-nested": "file:../_test_dependencies/svelte-nested"
   },
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "workspace:*",
diff --git a/packages/e2e-tests/prebundle-svelte-deps/__tests__/prebundle-svelte-deps.spec.ts b/packages/e2e-tests/prebundle-svelte-deps/__tests__/prebundle-svelte-deps.spec.ts
@@ -0,0 +1,42 @@
+import { browserLogs, getText, isBuild, readVitePrebundleMetadata } from '~utils';
+
+test('should not have failed requests', async () => {
+	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('#hybrid .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');
+});
+
+if (!isBuild) {
+	test('should optimize svelte dependencies', () => {
+		const metadataFile = readVitePrebundleMetadata();
+		const metadata = JSON.parse(metadataFile);
+		const optimizedPaths = Object.keys(metadata.optimized);
+		expect(optimizedPaths).toContain('e2e-test-dep-svelte-simple');
+		expect(optimizedPaths).toContain('e2e-test-dep-svelte-hybrid');
+		expect(optimizedPaths).toContain('e2e-test-dep-svelte-api-only');
+	});
+
+	test('should not optimize excluded svelte dependencies', () => {
+		const metadataFile = readVitePrebundleMetadata();
+		const metadata = JSON.parse(metadataFile);
+		const optimizedPaths = Object.keys(metadata.optimized);
+		expect(optimizedPaths).not.toContain('e2e-test-dep-scss-only');
+	});
+}
diff --git a/packages/e2e-tests/prebundle-svelte-deps/index.html b/packages/e2e-tests/prebundle-svelte-deps/index.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html lang="en">
+	<head>
+		<meta charset="UTF-8" />
+		<link rel="icon" type="image/png" href="/favicon.png" />
+		<meta name="viewport" content="width=device-width, initial-scale=1.0" />
+		<title>Vite + Svelte</title>
+	</head>
+	<body>
+		<div id="app"></div>
+		<script type="module" src="/src/main.js"></script>
+	</body>
+</html>
diff --git a/packages/e2e-tests/prebundle-svelte-deps/package.json b/packages/e2e-tests/prebundle-svelte-deps/package.json
@@ -0,0 +1,24 @@
+{
+  "name": "e2e-tests-prebundle-svelte-deps",
+  "private": true,
+  "version": "0.0.0",
+  "type": "module",
+  "scripts": {
+    "dev": "vite --force",
+    "build": "vite build",
+    "preview": "vite preview"
+  },
+  "dependencies": {
+    "e2e-test-dep-svelte-api-only": "file:../_test_dependencies/svelte-api-only",
+    "e2e-test-dep-svelte-hybrid": "file:../_test_dependencies/svelte-hybrid",
+    "e2e-test-dep-svelte-nested": "file:../_test_dependencies/svelte-nested",
+    "e2e-test-dep-svelte-simple": "file:../_test_dependencies/svelte-simple"
+  },
+  "devDependencies": {
+    "@sveltejs/vite-plugin-svelte": "workspace:*",
+    "sass": "^1.56.0",
+    "svelte": "^3.52.0",
+    "svelte-preprocess": "^4.10.7",
+    "vite": "^3.2.3"
+  }
+}
diff --git a/packages/e2e-tests/prebundle-svelte-deps/public/favicon.png b/packages/e2e-tests/prebundle-svelte-deps/public/favicon.png
diff --git a/packages/e2e-tests/prebundle-svelte-deps/src/App.svelte b/packages/e2e-tests/prebundle-svelte-deps/src/App.svelte
@@ -0,0 +1,22 @@
+<script>
+	import Hybrid from 'e2e-test-dep-svelte-hybrid';
+	import Simple from 'e2e-test-dep-svelte-simple';
+	import { Message as Nested } from 'e2e-test-dep-svelte-nested';
+	import { setSomeContext } from 'e2e-test-dep-svelte-api-only';
+	import { getContext } from 'svelte';
+	setSomeContext();
+	let apiOnlyLoaded = !!getContext('svelte-api-only');
+</script>
+
+<main>
+	<div id="hybrid">
+		<Hybrid />
+	</div>
+	<div id="simple">
+		<Simple />
+	</div>
+	<div id="nested">
+		<Nested id="message" message="nested" />
+	</div>
+	<div id="api-only">api loaded: {apiOnlyLoaded}</div>
+</main>
diff --git a/packages/e2e-tests/prebundle-svelte-deps/src/main.js b/packages/e2e-tests/prebundle-svelte-deps/src/main.js
@@ -0,0 +1,7 @@
+import App from './App.svelte';
+
+const app = new App({
+	target: document.body
+});
+
+export default app;
diff --git a/packages/e2e-tests/prebundle-svelte-deps/src/vite-env.d.ts b/packages/e2e-tests/prebundle-svelte-deps/src/vite-env.d.ts
@@ -0,0 +1,2 @@
+/// <reference types="svelte" />
+/// <reference types="vite/client" />
diff --git a/packages/e2e-tests/prebundle-svelte-deps/svelte.config.js b/packages/e2e-tests/prebundle-svelte-deps/svelte.config.js
@@ -0,0 +1,7 @@
+import preprocess from 'svelte-preprocess';
+export default {
+	preprocess: [preprocess()],
+	vitePlugin: {
+		prebundleSvelteLibraries: true
+	}
+};
diff --git a/packages/e2e-tests/prebundle-svelte-deps/vite.config.js b/packages/e2e-tests/prebundle-svelte-deps/vite.config.js
@@ -0,0 +1,17 @@
+import { defineConfig } from 'vite';
+import { svelte } from '@sveltejs/vite-plugin-svelte';
+
+// https://vitejs.dev/config/
+export default defineConfig({
+	plugins: [svelte()],
+	optimizeDeps: {
+		// TODO this must be excluded because nested has an scss dep that prebundle can't handle!
+		// figure out how to exclude it automatically or at least tell the user about it in a more friendly way
+		exclude: ['e2e-test-dep-scss-only']
+	},
+	build: {
+		// make build faster by skipping transforms and minification
+		target: 'esnext',
+		minify: false
+	}
+});
diff --git a/packages/e2e-tests/testUtils.ts b/packages/e2e-tests/testUtils.ts
@@ -268,3 +268,20 @@ export async function fetchPageText() {
 		throw new Error(`request to ${url} failed with ${res.status} - ${res.statusText}.`);
 	}
 }
+
+export function readVitePrebundleMetadata() {
+	const metadataPaths = [
+		'node_modules/.vite/_metadata.json',
+		'node_modules/.vite/deps/_metadata.json' // vite 2.9
+	];
+	for (const metadataPath of metadataPaths) {
+		try {
+			const vitePrebundleMetadata = path.resolve(testDir, metadataPath);
+			const metadataFile = fs.readFileSync(vitePrebundleMetadata, 'utf8');
+			return metadataFile;
+		} catch {
+			// ignore
+		}
+	}
+	throw new Error('Unable to find vite prebundle metadata');
+}
diff --git a/packages/e2e-tests/vite-ssr/package.json b/packages/e2e-tests/vite-ssr/package.json
@@ -14,7 +14,7 @@
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "compression": "^1.7.4",
     "cross-env": "^7.0.3",
-    "e2e-test-dep-esm-only": "workspace:*",
+    "e2e-test-dep-esm-only": "file:../_test_dependencies/esm-only",
     "express": "^4.18.2",
     "serve-static": "^1.15.0",
     "svelte": "^3.52.0",
diff --git a/packages/playground/big-component-library/package.json b/packages/playground/big-component-library/package.json
@@ -11,6 +11,7 @@
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "carbon-components-svelte": "^0.70.12",
+		"carbon-icons-svelte": "^11.4.0",
     "svelte": "^3.52.0",
     "vite": "^3.2.3"
   },
diff --git a/packages/playground/big-component-library/src/App.svelte b/packages/playground/big-component-library/src/App.svelte
@@ -1,7 +1,13 @@
 <script>
-	import { Accordion, AccordionItem, Button, DatePickerInput } from 'carbon-components-svelte';
-	import Link from 'carbon-components-svelte/src/Link/Link.svelte';
-	import DatePicker from 'carbon-components-svelte/src/DatePicker/DatePicker.svelte';
+	import {
+		Accordion,
+		AccordionItem,
+		Button,
+		DatePickerInput,
+		Link,
+		DatePicker
+	} from 'carbon-components-svelte';
+	import Save from 'carbon-icons-svelte/lib/Save.svelte';
 	import { assign } from 'lodash-es';
 	import toNumber from 'lodash-es/toNumber.js';
 	import toPath from 'lodash-es/toPath.js';
@@ -22,3 +28,4 @@
 	<AccordionItem title="Section 2">Content 2</AccordionItem>
 	<AccordionItem title="Section 3">Content 3</AccordionItem>
 </Accordion>
+<Save />
diff --git a/packages/playground/big-component-library/vite.config.js b/packages/playground/big-component-library/vite.config.js
@@ -2,11 +2,11 @@ import { svelte } from '@sveltejs/vite-plugin-svelte';
 import { defineConfig } from 'vite';
 
 export default defineConfig({
-	plugins: [
-		svelte({
-			experimental: {
-				prebundleSvelteLibraries: true
-			}
-		})
-	]
+	plugins: [svelte({ prebundleSvelteLibraries: true })],
+	optimizeDeps: {
+		// carbon-components-svelte is large, prebundle
+		include: ['carbon-components-svelte'],
+		// carbon-icons-svelte is huge and takes 12s to prebundle, better use deep imports for the icons you need
+		exclude: ['carbon-icons-svelte']
+	}
 });
diff --git a/packages/vite-plugin-svelte/src/index.ts b/packages/vite-plugin-svelte/src/index.ts
diff --git a/packages/vite-plugin-svelte/src/utils/optimizer.ts b/packages/vite-plugin-svelte/src/utils/optimizer.ts
diff --git a/packages/vite-plugin-svelte/src/utils/options.ts b/packages/vite-plugin-svelte/src/utils/options.ts
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'@sveltejs/vite-plugin-svelte': patch
 +---
++
 +fix(prebundleSvelteLibraries): avoid resolving via svelte field after a library has been prebundled
Original file line number	Diff line number	Diff line change
`@@ -12,8 +12,8 @@`
`12`	`12`	`"./package.json": "./package.json"`
`13`	`13`	`},`
`14`	`14`	`"dependencies": {`
`15`		`- "e2e-test-dep-svelte-simple": "workspace:*",`
`16`		`- "e2e-test-dep-cjs-and-esm": "workspace:*",`
`17`		`- "e2e-test-dep-scss-only": "workspace:*"`
	`15`	`+ "e2e-test-dep-svelte-simple": "file:../svelte-simple",`
	`16`	`+ "e2e-test-dep-cjs-and-esm": "file:../cjs-and-esm",`
	`17`	`+ "e2e-test-dep-scss-only": "file:../scss-only"`
`18`	`18`	`}`
`19`	`19`	`}`