fix: replace relative-require with our own resolveDependencyData (#254)

dominikg · web-flow · commit 1901b424f0ab · 2022-01-21T18:27:09.000+01:00
* fix: replace relative-require with our own resolveDependencyData to avoid errors when package.json isn't exported

* feat: implement simple cache for resolved svelte fields

* ci: use windows-2019 label to see if windows fail is caused by new github default

* ci: remove esbuild install calls, relax timeouts for ci and windows, add logging to globalSetup

* chore: add changeset
diff --git a/.changeset/five-houses-serve.md b/.changeset/five-houses-serve.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/vite-plugin-svelte': patch
+---
+
+do not use require-relative to resolve svelte field of libraries and cache resolved values (fixes #244)
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -34,9 +34,7 @@ jobs:
           cache: 'pnpm'
           cache-dependency-path: '**/pnpm-lock.yaml'
       - name: install
-        run: |
-          pnpm install --frozen-lockfile --prefer-offline --ignore-scripts
-          node node_modules/esbuild/install.js
+        run: pnpm install --frozen-lockfile --prefer-offline --ignore-scripts
       - name: build
         id: build
         run: pnpm run build
@@ -58,7 +56,7 @@ jobs:
       fail-fast: false
       matrix:
         node: [16]
-        os: [ubuntu-latest, macos-latest, windows-latest]
+        os: [ubuntu-latest, macos-latest, windows-2019]
         include:
           - node: 14
             os: ubuntu-latest
@@ -74,9 +72,7 @@ jobs:
           cache: 'pnpm'
           cache-dependency-path: '**/pnpm-lock.yaml'
       - name: install
-        run: |
-          pnpm install --frozen-lockfile --prefer-offline --ignore-scripts
-          node node_modules/esbuild/install.js
+        run: pnpm install --frozen-lockfile --prefer-offline --ignore-scripts
       - name: build
         run: pnpm run build:ci
       - name: run tests
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -32,9 +32,7 @@ jobs:
           cache: 'pnpm'
           cache-dependency-path: '**/pnpm-lock.yaml'
       - name: install
-        run: |
-          pnpm install --frozen-lockfile --prefer-offline --ignore-scripts
-          node node_modules/esbuild/install.js
+        run: pnpm install --frozen-lockfile --prefer-offline --ignore-scripts
 
       - name: Creating .npmrc
         run: |
diff --git a/package.json b/package.json
@@ -57,7 +57,7 @@
     "svelte": "^3.46.2",
     "ts-jest": "^27.1.3",
     "typescript": "^4.5.4",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   },
   "lint-staged": {
     "*.{js,ts,svelte,html,md,svx}": "eslint --fix",
@@ -74,7 +74,7 @@
   "pnpm": {
     "overrides": {
       "@sveltejs/vite-plugin-svelte": "workspace:*",
-      "vite": "^2.7.10",
+      "vite": "^2.7.13",
       "ansi-regex@>2.1.1 <5.0.1": "^5.0.1"
     }
   }
diff --git a/packages/e2e-tests/autoprefixer-browerslist/package.json b/packages/e2e-tests/autoprefixer-browerslist/package.json
@@ -17,6 +17,6 @@
     "postcss-load-config": "^3.1.1",
     "svelte": "^3.46.2",
     "svelte-preprocess": "^4.10.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/e2e-tests/configfile-custom/package.json b/packages/e2e-tests/configfile-custom/package.json
@@ -13,6 +13,6 @@
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "svelte": "^3.46.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/e2e-tests/configfile-esm/package.json b/packages/e2e-tests/configfile-esm/package.json
@@ -14,7 +14,7 @@
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "svelte": "^3.46.2",
     "svelte-preprocess": "^4.10.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   },
   "type": "module"
 }
diff --git a/packages/e2e-tests/custom-extensions/package.json b/packages/e2e-tests/custom-extensions/package.json
@@ -10,6 +10,6 @@
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "svelte": "^3.46.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/e2e-tests/env/package.json b/packages/e2e-tests/env/package.json
@@ -10,7 +10,7 @@
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "svelte": "^3.46.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   },
   "type": "module"
 }
diff --git a/packages/e2e-tests/hmr/package.json b/packages/e2e-tests/hmr/package.json
@@ -14,6 +14,6 @@
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "node-fetch": "^2.6.6",
     "svelte": "^3.46.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/e2e-tests/package-json-svelte-field/package.json b/packages/e2e-tests/package-json-svelte-field/package.json
@@ -14,6 +14,6 @@
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "svelte": "^3.46.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/e2e-tests/preprocess-with-vite/package.json b/packages/e2e-tests/preprocess-with-vite/package.json
@@ -12,6 +12,6 @@
     "sass": "^1.48.0",
     "stylus": "^0.56.0",
     "svelte": "^3.46.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/e2e-tests/svelte-preprocess/package.json b/packages/e2e-tests/svelte-preprocess/package.json
@@ -12,6 +12,6 @@
     "svelte": "^3.46.2",
     "svelte-preprocess": "^4.10.2",
     "typescript": "^4.5.4",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/e2e-tests/ts-type-import/package.json b/packages/e2e-tests/ts-type-import/package.json
@@ -12,6 +12,6 @@
     "@tsconfig/svelte": "^3.0.0",
     "@types/node": "^17.0.7",
     "svelte-preprocess": "^4.10.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/e2e-tests/vite-ssr-esm/package.json b/packages/e2e-tests/vite-ssr-esm/package.json
@@ -21,6 +21,6 @@
     "npm-run-all": "^4.1.5",
     "serve-static": "^1.14.2",
     "svelte": "^3.46.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/e2e-tests/vite-ssr/package.json b/packages/e2e-tests/vite-ssr/package.json
@@ -18,6 +18,6 @@
     "express": "^4.17.2",
     "serve-static": "^1.14.2",
     "svelte": "^3.46.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/playground/big-component-library/package.json b/packages/playground/big-component-library/package.json
@@ -12,6 +12,6 @@
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "carbon-components-svelte": "^0.52.0",
     "svelte": "^3.46.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/playground/big/package.json b/packages/playground/big/package.json
@@ -10,6 +10,6 @@
   "devDependencies": {
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "svelte": "^3.46.2",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/playground/optimizedeps-include/package.json b/packages/playground/optimizedeps-include/package.json
@@ -12,6 +12,6 @@
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "svelte": "^3.46.2",
     "tinro": "^0.6.10",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/playground/windicss/package.json b/packages/playground/windicss/package.json
@@ -14,7 +14,7 @@
     "@sveltejs/vite-plugin-svelte": "workspace:*",
     "diff-match-patch": "^1.0.5",
     "svelte": "^3.46.2",
-    "vite": "^2.7.12",
+    "vite": "^2.7.13",
     "vite-plugin-windicss": "^1.6.3"
   }
 }
diff --git a/packages/vite-plugin-svelte/package.json b/packages/vite-plugin-svelte/package.json
@@ -49,7 +49,6 @@
     "debug": "^4.3.3",
     "kleur": "^4.1.4",
     "magic-string": "^0.25.7",
-    "require-relative": "^0.8.7",
     "svelte-hmr": "^0.14.9"
   },
   "peerDependencies": {
@@ -70,6 +69,6 @@
     "rollup": "^2.64.0",
     "svelte": "^3.46.2",
     "tsup": "^5.11.11",
-    "vite": "^2.7.12"
+    "vite": "^2.7.13"
   }
 }
diff --git a/packages/vite-plugin-svelte/src/index.ts b/packages/vite-plugin-svelte/src/index.ts
@@ -27,7 +27,7 @@ export function svelte(inlineOptions?: Partial<Options>): Plugin {
 	}
 	validateInlineOptions(inlineOptions);
 	const cache = new VitePluginSvelteCache();
-	const pkg_export_errors = new Set();
+	const pkg_resolve_errors = new Set();
 	// updated in configResolved hook
 	let requestParser: IdParser;
 	let options: ResolvedOptions;
@@ -137,21 +137,13 @@ export function svelte(inlineOptions?: Partial<Options>): Plugin {
 			}
 
 			try {
-				const resolved = resolveViaPackageJsonSvelte(importee, importer);
+				const resolved = resolveViaPackageJsonSvelte(importee, importer, cache);
 				if (resolved) {
 					log.debug(`resolveId resolved ${resolved} via package.json svelte field of ${importee}`);
 					return resolved;
 				}
 			} catch (err) {
-				switch (err.code) {
-					case 'ERR_PACKAGE_PATH_NOT_EXPORTED':
-						pkg_export_errors.add(importee);
-						return null;
-					case 'MODULE_NOT_FOUND':
-						return null;
-					default:
-						throw err;
-				}
+				pkg_resolve_errors.add(importee);
 			}
 		},
 
@@ -206,10 +198,11 @@ export function svelte(inlineOptions?: Partial<Options>): Plugin {
 		 */
 		// TODO generateBundle isn't called by vite, is buildEnd enough or should it be logged once per violation in resolve
 		buildEnd() {
-			if (pkg_export_errors.size > 0) {
+			if (pkg_resolve_errors.size > 0) {
 				log.warn(
-					`The following packages did not export their \`package.json\` file so we could not check the "svelte" field. If you had difficulties importing svelte components from a package, then please contact the author and ask them to export the package.json file.`,
-					Array.from(pkg_export_errors, (s) => `- ${s}`).join('\n')
+					`vite-plugin-svelte was unable to find package.json of the following packages and wasn't able to resolve via their "svelte" field.
+					If you had difficulties importing svelte components from a package, then please contact the author and ask them to export the package.json file.
+					${Array.from(pkg_resolve_errors, (s) => `- ${s}`).join('\n')}`.replace(/\t/g, '')
 				);
 			}
 		}
diff --git a/packages/vite-plugin-svelte/src/utils/dependencies.ts b/packages/vite-plugin-svelte/src/utils/dependencies.ts
@@ -66,7 +66,10 @@ function getSvelteDependencies(
 	return result;
 }
 
-function resolveDependencyData(dep: string, localRequire: NodeRequire): DependencyData | void {
+export function resolveDependencyData(
+	dep: string,
+	localRequire: NodeRequire
+): DependencyData | void {
 	try {
 		const pkgJson = `${dep}/package.json`;
 		const pkg = localRequire(pkgJson);
@@ -166,7 +169,7 @@ const COMMON_PREFIXES_WITHOUT_SVELTE_FIELD = [
  * @param dependency {string}
  * @returns {boolean} true if it is a dependency without a svelte field
  */
-function is_common_without_svelte_field(dependency: string): boolean {
+export function is_common_without_svelte_field(dependency: string): boolean {
 	return (
 		COMMON_DEPENDENCIES_WITHOUT_SVELTE_FIELD.includes(dependency) ||
 		COMMON_PREFIXES_WITHOUT_SVELTE_FIELD.some(
diff --git a/packages/vite-plugin-svelte/src/utils/resolve.ts b/packages/vite-plugin-svelte/src/utils/resolve.ts
@@ -1,14 +1,29 @@
 import path from 'path';
-import fs from 'fs';
-// @ts-ignore
-import relative from 'require-relative';
+import { createRequire } from 'module';
+import { is_common_without_svelte_field, resolveDependencyData } from './dependencies';
+import { VitePluginSvelteCache } from './vite-plugin-svelte-cache';
 
-export function resolveViaPackageJsonSvelte(importee: string, importer?: string): string | void {
-	if (importer && isBareImport(importee)) {
-		const importeePkgFile = relative.resolve(`${importee}/package.json`, path.dirname(importer));
-		const importeePkg = JSON.parse(fs.readFileSync(importeePkgFile, { encoding: 'utf-8' }));
-		if (importeePkg.svelte) {
-			return path.resolve(path.dirname(importeePkgFile), importeePkg.svelte);
+export function resolveViaPackageJsonSvelte(
+	importee: string,
+	importer: string | undefined,
+	cache: VitePluginSvelteCache
+): string | void {
+	if (importer && isBareImport(importee) && !is_common_without_svelte_field(importee)) {
+		const cached = cache.getResolvedSvelteField(importee, importer);
+		if (cached) {
+			return cached;
+		}
+		const localRequire = createRequire(importer);
+		const pkgData = resolveDependencyData(importee, localRequire);
+		if (pkgData) {
+			const { pkg, dir } = pkgData;
+			if (pkg.svelte) {
+				const result = path.resolve(dir, pkg.svelte);
+				cache.setResolvedSvelteField(importee, importer, result);
+				return result;
+			}
+		} else {
+			throw new Error(`failed to resolve package.json of ${importee} imported by ${importer}`);
 		}
 	}
 }
diff --git a/packages/vite-plugin-svelte/src/utils/vite-plugin-svelte-cache.ts b/packages/vite-plugin-svelte/src/utils/vite-plugin-svelte-cache.ts
@@ -6,6 +6,7 @@ export class VitePluginSvelteCache {
 	private _js = new Map<string, Code>();
 	private _dependencies = new Map<string, string[]>();
 	private _dependants = new Map<string, Set<string>>();
+	private _resolvedSvelteFields = new Map<string, string>();
 
 	public update(compileData: CompileData) {
 		this.updateCSS(compileData);
@@ -80,4 +81,23 @@ export class VitePluginSvelteCache {
 		const dependants = this._dependants.get(path);
 		return dependants ? [...dependants] : [];
 	}
+
+	public getResolvedSvelteField(name: string, importer?: string): string | void {
+		return this._resolvedSvelteFields.get(this._getResolvedSvelteFieldKey(name, importer));
+	}
+
+	public setResolvedSvelteField(
+		importee: string,
+		importer: string | undefined = undefined,
+		resolvedSvelte: string
+	) {
+		this._resolvedSvelteFields.set(
+			this._getResolvedSvelteFieldKey(importee, importer),
+			resolvedSvelte
+		);
+	}
+
+	private _getResolvedSvelteFieldKey(importee: string, importer?: string): string {
+		return importer ? `${importer} > ${importee}` : importee;
+	}
 }
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/scripts/jestGlobalSetup.js b/scripts/jestGlobalSetup.js
diff --git a/scripts/jestPerTestSetup.ts b/scripts/jestPerTestSetup.ts

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'@sveltejs/vite-plugin-svelte': patch
 +---
++
 +do not use require-relative to resolve svelte field of libraries and cache resolved values (fixes #244)
Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,6 @@`
`17`	`17`	`"postcss-load-config": "^3.1.1",`
`18`	`18`	`"svelte": "^3.46.2",`
`19`	`19`	`"svelte-preprocess": "^4.10.2",`
`20`		`- "vite": "^2.7.12"`
	`20`	`+ "vite": "^2.7.13"`
`21`	`21`	`}`
`22`	`22`	`}`
Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,6 @@`
`13`	`13`	`"devDependencies": {`
`14`	`14`	`"@sveltejs/vite-plugin-svelte": "workspace:*",`
`15`	`15`	`"svelte": "^3.46.2",`
`16`		`- "vite": "^2.7.12"`
	`16`	`+ "vite": "^2.7.13"`
`17`	`17`	`}`
`18`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@`
`14`	`14`	`"@sveltejs/vite-plugin-svelte": "workspace:*",`
`15`	`15`	`"svelte": "^3.46.2",`
`16`	`16`	`"svelte-preprocess": "^4.10.2",`
`17`		`- "vite": "^2.7.12"`
	`17`	`+ "vite": "^2.7.13"`
`18`	`18`	`},`
`19`	`19`	`"type": "module"`
`20`	`20`	`}`
Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,6 @@`
`10`	`10`	`"devDependencies": {`
`11`	`11`	`"@sveltejs/vite-plugin-svelte": "workspace:*",`
`12`	`12`	`"svelte": "^3.46.2",`
`13`		`- "vite": "^2.7.12"`
	`13`	`+ "vite": "^2.7.13"`
`14`	`14`	`}`
`15`	`15`	`}`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@`
`10`	`10`	`"devDependencies": {`
`11`	`11`	`"@sveltejs/vite-plugin-svelte": "workspace:*",`
`12`	`12`	`"svelte": "^3.46.2",`
`13`		`- "vite": "^2.7.12"`
	`13`	`+ "vite": "^2.7.13"`
`14`	`14`	`},`
`15`	`15`	`"type": "module"`
`16`	`16`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,6 @@`
`14`	`14`	`"@sveltejs/vite-plugin-svelte": "workspace:*",`
`15`	`15`	`"node-fetch": "^2.6.6",`
`16`	`16`	`"svelte": "^3.46.2",`
`17`		`- "vite": "^2.7.12"`
	`17`	`+ "vite": "^2.7.13"`
`18`	`18`	`}`
`19`	`19`	`}`