From 165cdef666a1ac3bc55725b84bb00fbfc759b0f5 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Sun, 13 Nov 2022 19:31:33 -0500 Subject: [PATCH 1/6] feat(resolve): support "fallback array" in package exports field Closes #4439 More context: https://github.com/vitejs/vite/pull/10504 --- packages/vite/package.json | 2 +- packages/vite/src/node/plugins/resolve.ts | 150 ++++++++-------------- pnpm-lock.yaml | 14 +- 3 files changed, 60 insertions(+), 106 deletions(-) diff --git a/packages/vite/package.json b/packages/vite/package.json index 4cc39e7f156009..dd8dd089cd1267 100644 --- a/packages/vite/package.json +++ b/packages/vite/package.json @@ -110,7 +110,7 @@ "postcss-import": "^15.0.0", "postcss-load-config": "^4.0.1", "postcss-modules": "^5.0.0", - "resolve.exports": "^1.1.0", + "resolve.exports": "npm:@alloc/resolve.exports@^1.1.0", "sirv": "^2.0.2", "source-map-js": "^1.0.2", "source-map-support": "^0.5.21", diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 1c1c80f573e84d..b4f187d1c27daf 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -2,7 +2,7 @@ import fs from 'node:fs' import path from 'node:path' import colors from 'picocolors' import type { PartialResolvedId } from 'rollup' -import { resolve as _resolveExports } from 'resolve.exports' +import { resolveExports } from 'resolve.exports' import { hasESMSyntax } from 'mlly' import type { Plugin } from '../plugin' import { @@ -923,29 +923,28 @@ export function resolvePackageEntry( return cached } try { - let entryPoint: string | undefined | void + let entryPoints: string[] = [] - // resolve exports field with highest priority - // using https://github.com/lukeed/resolve.exports + // the exports field takes highest priority as described in + // https://nodejs.org/api/packages.html#package-entry-points if (data.exports) { - entryPoint = resolveExports(data, '.', options, targetWeb) - } - - // if exports resolved to .mjs, still resolve other fields. - // This is because .mjs files can technically import .cjs files which would - // make them invalid for pure ESM environments - so if other module/browser - // fields are present, prioritize those instead. - if ( - targetWeb && - options.browserField && - (!entryPoint || entryPoint.endsWith('.mjs')) - ) { + entryPoints = resolveExports( + data, + '.', + options, + getInlineConditions(options.conditions, targetWeb) + ) + if (!entryPoints.length) { + packageEntryFailure(id) + } + } else if (targetWeb && options.browserField) { // check browser field // https://github.com/defunctzombie/package-browser-field-spec const browserEntry = typeof data.browser === 'string' ? data.browser : isObject(data.browser) && data.browser['.'] + if (browserEntry) { // check if the package also has a "module" field. if ( @@ -968,34 +967,34 @@ export function resolvePackageEntry( const content = fs.readFileSync(resolvedBrowserEntry, 'utf-8') if (hasESMSyntax(content)) { // likely ESM, prefer browser - entryPoint = browserEntry + entryPoints[0] = browserEntry } else { // non-ESM, UMD or IIFE or CJS(!!! e.g. firebase 7.x), prefer module - entryPoint = data.module + entryPoints[0] = data.module } } } else { - entryPoint = browserEntry + entryPoints[0] = browserEntry } } } - if (!entryPoint || entryPoint.endsWith('.mjs')) { + if (!entryPoints[0]) { for (const field of options.mainFields) { if (field === 'browser') continue // already checked above if (typeof data[field] === 'string') { - entryPoint = data[field] + entryPoints[0] = data[field] break } } + entryPoints[0] ||= data.main } - entryPoint ||= data.main // try default entry when entry is not define // https://nodejs.org/api/modules.html#all-together - const entryPoints = entryPoint - ? [entryPoint] - : ['index.js', 'index.json', 'index.node'] + if (!entryPoints[0]) { + entryPoints = ['index.js', 'index.json', 'index.node'] + } for (let entry of entryPoints) { // make sure we don't get scripts when looking for sass @@ -1040,52 +1039,8 @@ function packageEntryFailure(id: string, details?: string) { ) } -const conditionalConditions = new Set(['production', 'development', 'module']) - -function resolveExports( - pkg: PackageData['data'], - key: string, - options: InternalResolveOptionsWithOverrideConditions, - targetWeb: boolean -) { - const overrideConditions = options.overrideConditions - ? new Set(options.overrideConditions) - : undefined - - const conditions = [] - if ( - (!overrideConditions || overrideConditions.has('production')) && - options.isProduction - ) { - conditions.push('production') - } - if ( - (!overrideConditions || overrideConditions.has('development')) && - !options.isProduction - ) { - conditions.push('development') - } - if ( - (!overrideConditions || overrideConditions.has('module')) && - !options.isRequire - ) { - conditions.push('module') - } - if (options.overrideConditions) { - conditions.push( - ...options.overrideConditions.filter((condition) => - conditionalConditions.has(condition) - ) - ) - } else if (options.conditions.length > 0) { - conditions.push(...options.conditions) - } - - return _resolveExports(pkg, key, { - browser: targetWeb && !conditions.includes('node'), - require: options.isRequire && !conditions.includes('import'), - conditions - }) +function getInlineConditions(conditions: string[], targetWeb: boolean) { + return targetWeb && !conditions.includes('node') ? ['browser'] : ['node'] } function resolveDeepImport( @@ -1098,56 +1053,55 @@ function resolveDeepImport( data }: PackageData, targetWeb: boolean, - options: InternalResolveOptions + options: InternalResolveOptionsWithOverrideConditions ): string | undefined { const cache = getResolvedCache(id, targetWeb) if (cache) { return cache } - let relativeId: string | undefined | void = id const { exports: exportsField, browser: browserField } = data + const { file, postfix } = splitFileAndPostfix(id) - // map relative based on exports data + let possibleFiles: string[] | undefined if (exportsField) { - if (isObject(exportsField) && !Array.isArray(exportsField)) { - // resolve without postfix (see #7098) - const { file, postfix } = splitFileAndPostfix(relativeId) - const exportsId = resolveExports(data, file, options, targetWeb) - if (exportsId !== undefined) { - relativeId = exportsId + postfix - } else { - relativeId = undefined - } - } else { - // not exposed - relativeId = undefined - } - if (!relativeId) { + // map relative based on exports data + possibleFiles = resolveExports( + data, + file, + options, + getInlineConditions(options.conditions, targetWeb), + options.overrideConditions + ) + if (!possibleFiles.length) { throw new Error( - `Package subpath '${relativeId}' is not defined by "exports" in ` + + `Package subpath '${file}' is not defined by "exports" in ` + `${path.join(dir, 'package.json')}.` ) } } else if (targetWeb && options.browserField && isObject(browserField)) { - // resolve without postfix (see #7098) - const { file, postfix } = splitFileAndPostfix(relativeId) const mapped = mapWithBrowserField(file, browserField) if (mapped) { - relativeId = mapped + postfix + possibleFiles = [mapped] } else if (mapped === false) { return (webResolvedImports[id] = browserExternalId) } } - if (relativeId) { - const resolved = tryFsResolve( - path.join(dir, relativeId), - options, - !exportsField, // try index only if no exports field - targetWeb + possibleFiles ||= [id] + if (possibleFiles[0]) { + let resolved: string | undefined + possibleFiles.some( + (file) => + (resolved = tryFsResolve( + path.join(dir, file), + options, + !exportsField, // try index only if no exports field + targetWeb + )) ) if (resolved) { + resolved += postfix isDebug && debug( `[node/deep-import] ${colors.cyan(id)} -> ${colors.dim(resolved)}` diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a7f8770e4973ee..3f1369b739fd2e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -260,7 +260,7 @@ importers: postcss-load-config: ^4.0.1 postcss-modules: ^5.0.0 resolve: ^1.22.1 - resolve.exports: ^1.1.0 + resolve.exports: npm:@alloc/resolve.exports@^1.1.0 rollup: ~3.3.0 sirv: ^2.0.2 source-map-js: ^1.0.2 @@ -323,7 +323,7 @@ importers: postcss-import: 15.0.0_postcss@8.4.19 postcss-load-config: 4.0.1_postcss@8.4.19 postcss-modules: 5.0.0_postcss@8.4.19 - resolve.exports: 1.1.0 + resolve.exports: /@alloc/resolve.exports/1.1.0 sirv: 2.0.2 source-map-js: 1.0.2 source-map-support: 0.5.21 @@ -1420,6 +1420,11 @@ packages: '@algolia/requester-common': 4.13.1 dev: true + /@alloc/resolve.exports/1.1.0: + resolution: {integrity: sha512-daZJ4gBXxPUgjWjtxRp+5mU9tV6k7cSG2iKFyiPZOTxRzMRFPEe8dcTuqP+zIVSTfFpN1/SCIOMMYeYA7GwQvQ==} + engines: {node: '>=10'} + dev: true + /@ampproject/remapping/2.2.0: resolution: {integrity: sha512-qRmjj8nj9qmLTQXXmaR1cck3UXSRMPrbsLJAasZpF+t3riI71BXed5ebIOYwQntykeZuhjsdweEc9BxH5Jc26w==} engines: {node: '>=6.0.0'} @@ -7772,11 +7777,6 @@ packages: resolution: {integrity: sha512-pb/MYmXstAkysRFx8piNI1tGFNQIFA3vkE3Gq4EuA1dF6gHp/+vgZqsCGJapvy8N3Q+4o7FwvquPJcnZ7RYy4g==} engines: {node: '>=4'} - /resolve.exports/1.1.0: - resolution: {integrity: sha512-J1l+Zxxp4XK3LUDZ9m60LRJF/mAe4z6a4xyabPHk7pvK5t35dACV32iIjJDFeWZFfZlO29w6SZ67knR0tHzJtQ==} - engines: {node: '>=10'} - dev: true - /resolve/1.17.0: resolution: {integrity: sha512-ic+7JYiV8Vi2yzQGFWOkiZD5Z9z7O2Zhm9XMaTxdJExKasieFCr+yXZ/WmXsckHiKl12ar0y6XiXDx3m4RHn1w==} dependencies: From 606f126f496386d501acf2ec7444152a895d8a53 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Mon, 14 Nov 2022 13:12:55 -0500 Subject: [PATCH 2/6] refactor: overrideConditions --- packages/vite/src/node/plugins/resolve.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index b4f187d1c27daf..6ff88dff49d541 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -579,21 +579,19 @@ function tryResolveFile( } } -export type InternalResolveOptionsWithOverrideConditions = - InternalResolveOptions & { - /** - * @deprecated In future, `conditions` will work like this. - * @internal - */ - overrideConditions?: string[] - } +export interface InternalNodeResolveOptions extends InternalResolveOptions { + /** + * When defined, only conditions defined in this array will be used. + */ + overrideConditions?: string[] +} export const idToPkgMap = new Map() export function tryNodeResolve( id: string, importer: string | null | undefined, - options: InternalResolveOptionsWithOverrideConditions, + options: InternalNodeResolveOptions, targetWeb: boolean, depsOptimizer?: DepsOptimizer, ssr?: boolean, @@ -1053,7 +1051,7 @@ function resolveDeepImport( data }: PackageData, targetWeb: boolean, - options: InternalResolveOptionsWithOverrideConditions + options: InternalNodeResolveOptions ): string | undefined { const cache = getResolvedCache(id, targetWeb) if (cache) { From a94200f853e94c8d0498c5bfbb6eea5dfd63e67d Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Mon, 14 Nov 2022 13:13:13 -0500 Subject: [PATCH 3/6] fix: add `module` condition by default --- packages/vite/src/node/plugins/resolve.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 6ff88dff49d541..796880b980c66a 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -1038,7 +1038,14 @@ function packageEntryFailure(id: string, details?: string) { } function getInlineConditions(conditions: string[], targetWeb: boolean) { - return targetWeb && !conditions.includes('node') ? ['browser'] : ['node'] + const inlineConditions = + targetWeb && !conditions.includes('node') ? ['browser'] : ['node'] + + // The "module" condition is no longer recommended, but some older + // packages may still use it. + inlineConditions.push('module') + + return inlineConditions } function resolveDeepImport( From 296fe46928341e8071313fad81a0dbe25060fd92 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Mon, 14 Nov 2022 13:14:08 -0500 Subject: [PATCH 4/6] fix: config bundling regression Bug introduced in #10683 Some packages use "require" instead of "default" for CJS entry --- packages/vite/src/node/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index e90bdd7150bced..77680d34f3c224 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -968,7 +968,7 @@ async function bundleConfigFile( mainFields: [], browserField: false, conditions: [], - overrideConditions: ['node'], + overrideConditions: ['node', 'require'], dedupe: [], extensions: DEFAULT_EXTENSIONS, preserveSymlinks: false From 4d3c6ac03cafd5014a0592b7480be4ddc34a5184 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Mon, 14 Nov 2022 13:32:13 -0500 Subject: [PATCH 5/6] fix: respect `overrideConditions` and `isRequire` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …in the `getInlineConditions` function. --- packages/vite/src/node/config.ts | 8 +--- packages/vite/src/node/plugins/resolve.ts | 51 ++++++++++++++++------- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 77680d34f3c224..23f4b8c40371e4 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -48,11 +48,7 @@ import { DEFAULT_MAIN_FIELDS, ENV_ENTRY } from './constants' -import type { - InternalResolveOptions, - InternalResolveOptionsWithOverrideConditions, - ResolveOptions -} from './plugins/resolve' +import type { InternalResolveOptions, ResolveOptions } from './plugins/resolve' import { resolvePlugin, tryNodeResolve } from './plugins/resolve' import type { LogLevel, Logger } from './logger' import { createLogger } from './logger' @@ -958,7 +954,7 @@ async function bundleConfigFile( { name: 'externalize-deps', setup(build) { - const options: InternalResolveOptionsWithOverrideConditions = { + const options: InternalResolveOptions = { root: path.dirname(fileName), isBuild: true, isProduction: true, diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 796880b980c66a..4934192697c898 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -107,6 +107,7 @@ export interface InternalResolveOptions extends Required { shouldExternalize?: (id: string) => boolean | undefined // Check this resolve is called from `hookNodeResolve` in SSR isHookNodeResolve?: boolean + overrideConditions?: string[] } export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { @@ -579,19 +580,12 @@ function tryResolveFile( } } -export interface InternalNodeResolveOptions extends InternalResolveOptions { - /** - * When defined, only conditions defined in this array will be used. - */ - overrideConditions?: string[] -} - export const idToPkgMap = new Map() export function tryNodeResolve( id: string, importer: string | null | undefined, - options: InternalNodeResolveOptions, + options: InternalResolveOptions, targetWeb: boolean, depsOptimizer?: DepsOptimizer, ssr?: boolean, @@ -930,7 +924,8 @@ export function resolvePackageEntry( data, '.', options, - getInlineConditions(options.conditions, targetWeb) + getInlineConditions(options, targetWeb), + options.overrideConditions ) if (!entryPoints.length) { packageEntryFailure(id) @@ -1037,13 +1032,39 @@ function packageEntryFailure(id: string, details?: string) { ) } -function getInlineConditions(conditions: string[], targetWeb: boolean) { - const inlineConditions = - targetWeb && !conditions.includes('node') ? ['browser'] : ['node'] +/** + * This generates conditions that aren't inferred by `resolveExports` + * from the `options` object. + */ +function getInlineConditions( + options: InternalResolveOptions, + targetWeb: boolean +) { + const inlineConditions: string[] = [] + + const conditions: readonly string[] = + options.overrideConditions || options.conditions + + if (targetWeb) { + if (!conditions.includes('node')) { + inlineConditions.push('browser') + } + } else if (!conditions.includes('browser')) { + inlineConditions.push('node') + } // The "module" condition is no longer recommended, but some older // packages may still use it. - inlineConditions.push('module') + if (!options.isRequire && !conditions.includes('require')) { + inlineConditions.push('module') + } + + // The "overrideConditions" array can add arbitrary conditions. + options.overrideConditions?.forEach((condition) => { + if (!inlineConditions.includes(condition)) { + inlineConditions.push(condition) + } + }) return inlineConditions } @@ -1058,7 +1079,7 @@ function resolveDeepImport( data }: PackageData, targetWeb: boolean, - options: InternalNodeResolveOptions + options: InternalResolveOptions ): string | undefined { const cache = getResolvedCache(id, targetWeb) if (cache) { @@ -1075,7 +1096,7 @@ function resolveDeepImport( data, file, options, - getInlineConditions(options.conditions, targetWeb), + getInlineConditions(options, targetWeb), options.overrideConditions ) if (!possibleFiles.length) { From bb4fbd0dbd387e0d0e4769f44c15ccbf5927e220 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Mon, 14 Nov 2022 14:17:57 -0500 Subject: [PATCH 6/6] fix: try `resolveExports` call with postfix included MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …and avoid breaking certain `tryFsResolve` calls too (like with `es5-ext`) --- packages/vite/src/node/plugins/resolve.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 4934192697c898..5cabc36c8d69d4 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -1099,6 +1099,19 @@ function resolveDeepImport( getInlineConditions(options, targetWeb), options.overrideConditions ) + if (postfix) { + if (possibleFiles.length) { + possibleFiles = possibleFiles.map((f) => f + postfix) + } else { + possibleFiles = resolveExports( + data, + file + postfix, + options, + getInlineConditions(options, targetWeb), + options.overrideConditions + ) + } + } if (!possibleFiles.length) { throw new Error( `Package subpath '${file}' is not defined by "exports" in ` + @@ -1108,7 +1121,7 @@ function resolveDeepImport( } else if (targetWeb && options.browserField && isObject(browserField)) { const mapped = mapWithBrowserField(file, browserField) if (mapped) { - possibleFiles = [mapped] + possibleFiles = [mapped + postfix] } else if (mapped === false) { return (webResolvedImports[id] = browserExternalId) } @@ -1127,7 +1140,6 @@ function resolveDeepImport( )) ) if (resolved) { - resolved += postfix isDebug && debug( `[node/deep-import] ${colors.cyan(id)} -> ${colors.dim(resolved)}`