refactor(resolve): prefer exports condition over svelte field (#747)

* refactor(resolve): prefer exports condition over svelte field

* chore: add changeset

* refactor: use vite mainFields resolve instead of our custom implementation, log packages while scanning deps instead

Author: dominikg

.changeset/ten-tips-sip.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/vite-plugin-svelte': major
+---
+
+breaking: prefer svelte exports condition over package.json svelte field

docs/faq.md

@@ -209,13 +209,12 @@ module.exports = {
 
 <!-- the following header generates an anchor that is used in logging, do not modify!-->
 
-### conflicts in svelte resolve
+### missing exports condition
 
 | If you see a warning logged for this when using a Svelte library, please tell the library maintainers.
 
-In the past, Svelte recommended using the custom `svelte` field in `package.json` to allow libraries to point at `.svelte` source files.
-This field requires a custom implementation to resolve, so you have to use a bundler plugin and this plugin needs to implement resolving.
-Since then, Node has added support for [conditional exports](https://nodejs.org/api/packages.html#conditional-exports), which have more generic support in bundlers and Node itself. So to increase the compatibility with the wider ecosystem and reduce the implementation needs for current and future bundler plugins, it is recommended that packages use the `svelte` exports condition.
+Using the `svelte` field in `package.json` to point at `.svelte` source files is **deprecated** and you must use a `svelte` [export condition](https://nodejs.org/api/packages.html#conditional-exports).
+vite-plugin-svelte 3 still resolves it as a fallback, but in a future major release this is going to be removed and without exports condition resolving the library is going to fail.
 
 Example:
 

packages/e2e-tests/_test_dependencies/svelte-hybrid/package.json

@@ -10,6 +10,13 @@
     "index.js",
     "cli.js"
   ],
+  "exports": {
+    ".": {
+      "import": {
+        "svelte": "./index.js"
+      }
+    }
+  },
   "dependencies": {
     "@types/node": "^18.17.15",
     "e2e-test-dep-cjs-only": "file:../cjs-only"

packages/e2e-tests/_test_dependencies/svelte-nested/package.json

@@ -9,7 +9,12 @@
     "src"
   ],
   "exports": {
-    "./package.json": "./package.json"
+    "./package.json": "./package.json",
+    ".": {
+      "import": {
+        "svelte": "./src/index.js"
+      }
+    }
   },
   "dependencies": {
     "e2e-test-dep-svelte-simple": "file:../svelte-simple",

packages/e2e-tests/_test_dependencies/svelte-simple/package.json

@@ -8,6 +8,13 @@
     "src",
     "index.js"
   ],
+  "exports": {
+    ".": {
+      "import": {
+        "svelte": "./index.js"
+      }
+    }
+  },
   "dependencies": {
     "e2e-test-dep-cjs-only": "file:../cjs-only"
   },

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

@@ -2,7 +2,6 @@ import fs from 'node:fs';
 
 import { svelteInspector } from '@sveltejs/vite-plugin-svelte-inspector';
 
-import { isDepExcluded } from 'vitefu';
 import { handleHotUpdate } from './handle-hot-update.js';
 import { log, logCompilerWarnings } from './utils/log.js';
 import { createCompileSvelte } from './utils/compile.js';
@@ -16,13 +15,10 @@ import {
 } from './utils/options.js';
 
 import { ensureWatchedFile, setupWatchers } from './utils/watch.js';
-import { resolveViaPackageJsonSvelte } from './utils/resolve.js';
-
 import { toRollupError } from './utils/error.js';
 import { saveSvelteMetadata } from './utils/optimizer.js';
 import { VitePluginSvelteCache } from './utils/vite-plugin-svelte-cache.js';
 import { loadRaw } from './utils/load-raw.js';
-import { FAQ_LINK_CONFLICTS_IN_SVELTE_RESOLVE } from './utils/constants.js';
 
 /** @type {import('./index.d.ts').svelte} */
 export function svelte(inlineOptions) {
@@ -38,13 +34,9 @@ export function svelte(inlineOptions) {
 	let options;
 	/** @type {import('vite').ResolvedConfig} */
 	let viteConfig;
-
 	/** @type {import('./types/compile.d.ts').CompileSvelte} */
 	let compileSvelte;
 	/* eslint-enable no-unused-vars */
-
-	/** @type {Set<string>} */
-	let packagesWithResolveWarnings;
 	/** @type {import('./types/plugin-api.d.ts').PluginAPI} */
 	const api = {};
 	/** @type {import('vite').Plugin[]} */
@@ -81,7 +73,6 @@ export function svelte(inlineOptions) {
 			},
 
 			async buildStart() {
-				packagesWithResolveWarnings = new Set();
 				if (!options.prebundleSvelteLibraries) return;
 				const isSvelteMetadataChanged = await saveSvelteMetadata(viteConfig.cacheDir, options);
 				if (isSvelteMetadataChanged) {
@@ -138,57 +129,6 @@ export function svelte(inlineOptions) {
 						return svelteRequest.cssId;
 					}
 				}
-
-				//@ts-expect-error scan
-				const scan = !!opts?.scan; // scanner phase of optimizeDeps
-				const isPrebundled =
-					options.prebundleSvelteLibraries &&
-					viteConfig.optimizeDeps?.disabled !== true &&
-					viteConfig.optimizeDeps?.disabled !== (options.isBuild ? 'build' : 'dev') &&
-					!isDepExcluded(importee, viteConfig.optimizeDeps?.exclude ?? []);
-				// for prebundled libraries we let vite resolve the prebundling result
-				// for ssr, during scanning and non-prebundled, we do it
-				if (ssr || scan || !isPrebundled) {
-					try {
-						const isFirstResolve = !cache.hasResolvedSvelteField(importee, importer);
-						const resolved = await resolveViaPackageJsonSvelte(importee, importer, cache);
-						if (isFirstResolve && resolved) {
-							const packageInfo = await cache.getPackageInfo(resolved);
-							const packageVersion = `${packageInfo.name}@${packageInfo.version}`;
-							log.debug.once(
-								`resolveId resolved ${importee} to ${resolved} via package.json svelte field of ${packageVersion}`
-							);
-
-							try {
-								const viteResolved = (
-									await this.resolve(importee, importer, { ...opts, skipSelf: true })
-								)?.id;
-								if (resolved !== viteResolved) {
-									packagesWithResolveWarnings.add(packageVersion);
-									log.debug.enabled &&
-										log.debug.once(
-											`resolve difference for ${packageVersion} ${importee} - svelte: "${resolved}", vite: "${viteResolved}"`
-										);
-								}
-							} catch (e) {
-								packagesWithResolveWarnings.add(packageVersion);
-								log.debug.enabled &&
-									log.debug.once(
-										`resolve error for ${packageVersion} ${importee} - svelte: "${resolved}", vite: ERROR`,
-										e
-									);
-							}
-						}
-						return resolved;
-					} catch (e) {
-						log.debug.once(
-							`error trying to resolve ${importee} from ${importer} via package.json svelte field `,
-							e
-						);
-						// this error most likely happens due to non-svelte related importee/importers so swallow it here
-						// in case it really way a svelte library, users will notice anyway. (lib not working due to failed resolve)
-					}
-				}
 			},
 
 			async transform(code, id, opts) {
@@ -239,16 +179,6 @@ export function svelte(inlineOptions) {
 			},
 			async buildEnd() {
 				await options.stats?.finishAll();
-				if (
-					!options.experimental?.disableSvelteResolveWarnings &&
-					packagesWithResolveWarnings?.size > 0
-				) {
-					log.warn(
-						`WARNING: The following packages use a svelte resolve configuration in package.json that has conflicting results and is going to cause problems future.\n\n${[
-							...packagesWithResolveWarnings
-						].join('\n')}\n\nPlease see ${FAQ_LINK_CONFLICTS_IN_SVELTE_RESOLVE} for details.`
-					);
-				}
 			}
 		},
 		svelteInspector()

packages/vite-plugin-svelte/src/utils/constants.js

@@ -22,5 +22,5 @@ export const SVELTE_HMR_IMPORTS = [
 
 export const SVELTE_EXPORT_CONDITIONS = ['svelte'];
 
-export const FAQ_LINK_CONFLICTS_IN_SVELTE_RESOLVE =
-	'https://github.com/sveltejs/vite-plugin-svelte/blob/main/docs/faq.md#conflicts-in-svelte-resolve';
+export const FAQ_LINK_MISSING_EXPORTS_CONDITION =
+	'https://github.com/sveltejs/vite-plugin-svelte/blob/main/docs/faq.md#missing-exports-condition';

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

@@ -3,6 +3,7 @@ import { normalizePath } from 'vite';
 import { isDebugNamespaceEnabled, log } from './log.js';
 import { loadSvelteConfig } from './load-svelte-config.js';
 import {
+	FAQ_LINK_MISSING_EXPORTS_CONDITION,
 	SVELTE_EXPORT_CONDITIONS,
 	SVELTE_HMR_IMPORTS,
 	SVELTE_IMPORTS,
@@ -481,6 +482,7 @@ function validateViteConfig(extraViteConfig, config, options) {
  */
 async function buildExtraConfigForDependencies(options, config) {
 	// extra handling for svelte dependencies in the project
+	const packagesWithoutSvelteExportsCondition = new Set();
 	const depsConfig = await crawlFrameworkPkgs({
 		root: options.root,
 		isBuild: options.isBuild,
@@ -496,7 +498,11 @@ async function buildExtraConfigForDependencies(options, config) {
 					return value;
 				});
 			}
-			return hasSvelteCondition || !!pkgJson.svelte;
+			const hasSvelteField = !!pkgJson.svelte;
+			if (hasSvelteField && !hasSvelteCondition) {
+				packagesWithoutSvelteExportsCondition.add(`${pkgJson.name}@${pkgJson.version}`);
+			}
+			return hasSvelteCondition || hasSvelteField;
 		},
 		isSemiFrameworkPkgByJson(pkgJson) {
 			return !!pkgJson.dependencies?.svelte || !!pkgJson.peerDependencies?.svelte;
@@ -510,7 +516,16 @@ async function buildExtraConfigForDependencies(options, config) {
 			}
 		}
 	});
-
+	if (
+		!options.experimental?.disableSvelteResolveWarnings &&
+		packagesWithoutSvelteExportsCondition?.size > 0
+	) {
+		log.warn(
+			`WARNING: The following packages have a svelte field in their package.json but no exports condition for svelte.\n\n${[
+				...packagesWithoutSvelteExportsCondition
+			].join('\n')}\n\nPlease see ${FAQ_LINK_MISSING_EXPORTS_CONDITION} for details.`
+		);
+	}
 	log.debug('extra config for dependencies generated by vitefu', depsConfig);
 
 	if (options.prebundleSvelteLibraries) {

packages/vite-plugin-svelte/src/utils/resolve.js

@@ -24,8 +24,6 @@ export class VitePluginSvelteCache {
 	#dependencies = new Map();
 	/** @type {Map<string, Set<string>>} */
 	#dependants = new Map();
-	/** @type {Map<string, string>} */
-	#resolvedSvelteFields = new Map();
 	/** @type {Map<string, any>} */
 	#errors = new Map();
 	/** @type {PackageInfo[]} */
@@ -168,45 +166,6 @@ export class VitePluginSvelteCache {
 		return dependants ? [...dependants] : [];
 	}
 
-	/**
-	 * @param {string} name
-	 * @param {string} [importer]
-	 * @returns {string|void}
-	 */
-	getResolvedSvelteField(name, importer) {
-		return this.#resolvedSvelteFields.get(this.#getResolvedSvelteFieldKey(name, importer));
-	}
-
-	/**
-	 * @param {string} name
-	 * @param {string} [importer]
-	 * @returns {boolean}
-	 */
-	hasResolvedSvelteField(name, importer) {
-		return this.#resolvedSvelteFields.has(this.#getResolvedSvelteFieldKey(name, importer));
-	}
-	/**
-	 *
-	 * @param {string} importee
-	 * @param {string | undefined} importer
-	 * @param {string} resolvedSvelte
-	 */
-	setResolvedSvelteField(importee, importer, resolvedSvelte) {
-		this.#resolvedSvelteFields.set(
-			this.#getResolvedSvelteFieldKey(importee, importer),
-			resolvedSvelte
-		);
-	}
-
-	/**
-	 * @param {string} importee
-	 * @param {string | undefined} importer
-	 * @returns {string}
-	 */
-	#getResolvedSvelteFieldKey(importee, importer) {
-		return importer ? `${importer} > ${importee}` : importee;
-	}
-
 	/**
 	 * @param {string} file
 	 * @returns {Promise<PackageInfo>}