feat: log warning if svelte field causes difference in resolve (#510)

* feat: log deprecation warning if svelte field causes difference in resolve

* perf: only compare svelte resolve with vite resolve on first occurrence

* fix: add vite resolve error to log

* docs: add faq entry for svelte field deprecation

* fix: add FAQ entry and link to it from logs, refactor logging code a bit

* fix: log resolve difference

* refactor: log reduced warning at buildEnd, log details during debug only. add flag to disable warning log.

* docs: update wording and improve exports map example

* Apply suggestions from code review

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* docs: use more neutral language for resolve differences, svelte field isn't deprecated yet

* Apply suggestions from code review

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* docs: remove glob exports from example, add note about deep exports

* docs: spelling

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* chore: fix changeset to use minor

---------

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

.changeset/cold-horses-begin.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/vite-plugin-svelte': minor
+---
+
+log warnings for packages that use the `svelte` field to resolve Svelte files differently than standard Vite resolve

docs/config.md

@@ -403,3 +403,10 @@ export default {
     rawWarnings: Warning[]; // raw compiler output
   };
   ```
+
+### disableSvelteResolveWarnings
+
+- **Type** `boolean`
+- **Default:** `false`
+
+  disable svelte resolve warnings. Note: this is highly discouraged and you should instead fix these packages which will break in the future.

docs/faq.md

@@ -208,3 +208,34 @@ module.exports = {
 +    }
 }
 ```
+
+<!-- the following header generates an anchor that is used in logging, do not modify!-->
+
+### conflicts in svelte resolve
+
+| 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.
+
+Example:
+
+```diff
+// package.json
+  "files": ["dist"],
+  "svelte": "dist/index.js",
++ "exports": {
++   ".": {
++     "svelte": "./dist/index.js"
++   }
+  }
+```
+
+You can also add individual exports of .svelte files in the exports map which gives users a choice to also use deep imports.
+See the faq about [vite and prebundling](#what-is-going-on-with-vite-and-pre-bundling-dependencies) why they can be useful at times.
+
+> Library authors are highly encouraged to update their packages to add the new exports condition as outlined above. Check out
+> [svelte-package](https://kit.svelte.dev/docs/packaging) which already supports it.
+>
+> For backwards compatibility, you can keep the `svelte` field in addition to the `exports` condition. But make sure that both always resolve to the same files.

packages/e2e-tests/_test_dependencies/svelte-exports-simple/index.svelte.js

@@ -0,0 +1 @@
+export { default as Dependency } from './src/components/Dependency.svelte';

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

@@ -8,8 +8,10 @@
   "type": "module",
   "files": [
     "src",
-    "index.js"
+    "index.js",
+		"index.svelte.js"
   ],
+	"svelte": "./index.svelte.js",
 	"exports": {
 		"./*": {
 			"svelte": "./src/components/*"

packages/e2e-tests/resolve-exports-svelte/vite.config.js

@@ -3,5 +3,9 @@ import { svelte } from '@sveltejs/vite-plugin-svelte';
 
 // https://vitejs.dev/config/
 export default defineConfig({
-	plugins: [svelte({ compilerOptions: { css: 'none' } })]
+	plugins: [
+		svelte({
+			compilerOptions: { css: 'none' }
+		})
+	]
 });

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

@@ -32,6 +32,7 @@ import { saveSvelteMetadata } from './utils/optimizer';
 import { svelteInspector } from './ui/inspector/plugin';
 import { VitePluginSvelteCache } from './utils/vite-plugin-svelte-cache';
 import { loadRaw } from './utils/load-raw';
+import { FAQ_LINK_CONFLICTS_IN_SVELTE_RESOLVE } from './utils/constants';
 
 interface PluginAPI {
 	/**
@@ -61,6 +62,7 @@ export function svelte(inlineOptions?: Partial<Options>): Plugin[] {
 	/* eslint-enable no-unused-vars */
 
 	let resolvedSvelteSSR: Promise<PartialResolvedId | null>;
+	let packagesWithResolveWarnings: Set<string>;
 	const api: PluginAPI = {};
 	const plugins: Plugin[] = [
 		{
@@ -84,7 +86,7 @@ export function svelte(inlineOptions?: Partial<Options>): Plugin[] {
 			},
 
 			async configResolved(config) {
-				options = resolveOptions(options, config);
+				options = resolveOptions(options, config, cache);
 				patchResolvedViteConfig(config, options);
 				requestParser = buildIdParser(options);
 				compileSvelte = createCompileSvelte(options);
@@ -95,6 +97,7 @@ export function svelte(inlineOptions?: Partial<Options>): Plugin[] {
 			},
 
 			async buildStart() {
+				packagesWithResolveWarnings = new Set<string>();
 				if (!options.prebundleSvelteLibraries) return;
 				const isSvelteMetadataChanged = await saveSvelteMetadata(viteConfig.cacheDir, options);
 				if (isSvelteMetadataChanged) {
@@ -176,13 +179,36 @@ export function svelte(inlineOptions?: Partial<Options>): Plugin[] {
 				// 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 (resolved) {
-							log.debug(
-								`resolveId resolved ${resolved} via package.json svelte field of ${importee}`
+						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}`
 							);
-							return resolved;
+
+							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 `,
@@ -236,6 +262,16 @@ export function svelte(inlineOptions?: Partial<Options>): Plugin[] {
 			},
 			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.`
+					);
+				}
 			}
 		}
 	];

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

@@ -20,3 +20,6 @@ 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';

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

@@ -79,7 +79,7 @@ function createLogger(level: string): LogFn {
 	const logFn: LogFn = _log.bind(null, logger) as LogFn;
 	const logged = new Set<String>();
 	const once = function (message: string, payload?: any, namespace?: string) {
-		if (logged.has(message)) {
+		if (!logger.enabled || logged.has(message)) {
 			return;
 		}
 		logged.add(message);

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

@@ -34,6 +34,7 @@ import {
 
 import { isCommonDepWithoutSvelteField } from './dependencies';
 import { VitePluginSvelteStats } from './vite-plugin-svelte-stats';
+import { VitePluginSvelteCache } from './vite-plugin-svelte-cache';
 
 const allowedPluginOptions = new Set([
 	'include',
@@ -178,7 +179,8 @@ function mergeConfigs<T>(...configs: (Partial<T> | undefined)[]): T {
 // also validates the final config.
 export function resolveOptions(
 	preResolveOptions: PreResolvedOptions,
-	viteConfig: ResolvedConfig
+	viteConfig: ResolvedConfig,
+	cache: VitePluginSvelteCache
 ): ResolvedOptions {
 	const css = preResolveOptions.emitCss ? 'external' : 'injected';
 	const defaultOptions: Partial<Options> = {
@@ -207,7 +209,7 @@ export function resolveOptions(
 	enforceOptionsForProduction(merged);
 	// mergeConfigs would mangle functions on the stats class, so do this afterwards
 	if (log.debug.enabled && isDebugNamespaceEnabled('stats')) {
-		merged.stats = new VitePluginSvelteStats();
+		merged.stats = new VitePluginSvelteStats(cache);
 	}
 	return merged;
 }
@@ -721,6 +723,13 @@ export interface ExperimentalOptions {
 	 *
 	 */
 	sendWarningsToBrowser?: boolean;
+
+	/**
+	 * disable svelte field resolve warnings
+	 *
+	 * @default false
+	 */
+	disableSvelteResolveWarnings?: boolean;
 }
 
 export interface InspectorOptions {

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

@@ -1,5 +1,17 @@
 import { SvelteRequest } from './id';
 import { Code, CompileData } from './compile';
+import { readFileSync } from 'fs';
+import { dirname } from 'path';
+//eslint-disable-next-line node/no-missing-import
+import { findClosestPkgJsonPath } from 'vitefu';
+import { normalizePath } from 'vite';
+
+interface PackageInfo {
+	name: string;
+	version: string;
+	svelte?: string;
+	path: string;
+}
 
 export class VitePluginSvelteCache {
 	private _css = new Map<string, Code>();
@@ -8,6 +20,7 @@ export class VitePluginSvelteCache {
 	private _dependants = new Map<string, Set<string>>();
 	private _resolvedSvelteFields = new Map<string, string>();
 	private _errors = new Map<string, any>();
+	private _packageInfos: PackageInfo[] = [];
 
 	public update(compileData: CompileData) {
 		this._errors.delete(compileData.normalizedFilename);
@@ -110,6 +123,9 @@ export class VitePluginSvelteCache {
 		return this._resolvedSvelteFields.get(this._getResolvedSvelteFieldKey(name, importer));
 	}
 
+	public hasResolvedSvelteField(name: string, importer?: string) {
+		return this._resolvedSvelteFields.has(this._getResolvedSvelteFieldKey(name, importer));
+	}
 	public setResolvedSvelteField(
 		importee: string,
 		importer: string | undefined = undefined,
@@ -124,4 +140,43 @@ export class VitePluginSvelteCache {
 	private _getResolvedSvelteFieldKey(importee: string, importer?: string): string {
 		return importer ? `${importer} > ${importee}` : importee;
 	}
+
+	public async getPackageInfo(file: string): Promise<PackageInfo> {
+		let info = this._packageInfos.find((pi) => file.startsWith(pi.path));
+		if (!info) {
+			info = await findPackageInfo(file);
+			this._packageInfos.push(info);
+		}
+		return info;
+	}
+}
+
+/**
+ * utility to get some info from the closest package.json with a "name" set
+ *
+ * @param {string} file to find info for
+ * @returns {PackageInfo}
+ */
+async function findPackageInfo(file: string): Promise<PackageInfo> {
+	const info: PackageInfo = {
+		name: '$unknown',
+		version: '0.0.0-unknown',
+		path: '$unknown'
+	};
+	let path = await findClosestPkgJsonPath(file, (pkgPath) => {
+		const pkg = JSON.parse(readFileSync(pkgPath, 'utf-8'));
+		if (pkg.name != null) {
+			info.name = pkg.name;
+			if (pkg.version != null) {
+				info.version = pkg.version;
+			}
+			info.svelte = pkg.svelte;
+			return true;
+		}
+		return false;
+	});
+	// return normalized path with appended '/' so .startsWith works for future file checks
+	path = normalizePath(dirname(path ?? file)) + '/';
+	info.path = path;
+	return info;
 }

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

@@ -1,10 +1,7 @@
 import { log } from './log';
-//eslint-disable-next-line node/no-missing-import
-import { findClosestPkgJsonPath } from 'vitefu';
-import { readFileSync } from 'fs';
-import { dirname } from 'path';
 import { performance } from 'perf_hooks';
 import { normalizePath } from 'vite';
+import { VitePluginSvelteCache } from './vite-plugin-svelte-cache';
 
 interface Stat {
 	file: string;
@@ -87,31 +84,13 @@ function formatPackageStats(pkgStats: PackageStats[]) {
 	return table;
 }
 
-/**
- * utility to get the package name a file belongs to
- *
- * @param {string} file to find package for
- * @returns {path:string,name:string} tuple of path,name where name is the parsed package name and path is the normalized path to it
- */
-async function getClosestNamedPackage(file: string): Promise<{ name: string; path: string }> {
-	let name = '$unknown';
-	let path = await findClosestPkgJsonPath(file, (pkgPath) => {
-		const pkg = JSON.parse(readFileSync(pkgPath, 'utf-8'));
-		if (pkg.name != null) {
-			name = pkg.name;
-			return true;
-		}
-		return false;
-	});
-	// return normalized path with appended '/' so .startsWith works for future file checks
-	path = normalizePath(dirname(path ?? file)) + '/';
-	return { name, path };
-}
-
 export class VitePluginSvelteStats {
 	// package directory -> package name
-	private _packages: { path: string; name: string }[] = [];
+	private _cache: VitePluginSvelteCache;
 	private _collections: StatCollection[] = [];
+	constructor(cache: VitePluginSvelteCache) {
+		this._cache = cache;
+	}
 	startCollection(name: string, opts?: Partial<CollectionOptions>) {
 		const options = {
 			...defaultCollectionOptions,
@@ -190,12 +169,7 @@ export class VitePluginSvelteStats {
 	private async _aggregateStatsResult(collection: StatCollection) {
 		const stats = collection.stats;
 		for (const stat of stats) {
-			let pkg = this._packages.find((p) => stat.file.startsWith(p.path));
-			if (!pkg) {
-				pkg = await getClosestNamedPackage(stat.file);
-				this._packages.push(pkg);
-			}
-			stat.pkg = pkg.name;
+			stat.pkg = (await this._cache.getPackageInfo(stat.file)).name;
 		}
 
 		// group stats