From 72e83cf71be330deb28fd5f383725c4fbd0200ec Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 11 Sep 2022 18:04:49 +0900 Subject: [PATCH 1/4] refactor: normalize resolveOptions in resolveConfig --- packages/vite/src/node/config.ts | 16 +++++++++----- packages/vite/src/node/plugins/resolve.ts | 13 +++++------ .../vite/src/node/plugins/ssrRequireHook.ts | 2 +- packages/vite/src/node/ssr/index.ts | 4 ++-- packages/vite/src/node/ssr/ssrExternal.ts | 22 +++++++------------ packages/vite/src/node/ssr/ssrModuleLoader.ts | 7 +++--- 6 files changed, 32 insertions(+), 32 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 829c47fee07abf..6e02df35663c96 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -43,6 +43,8 @@ import { CLIENT_ENTRY, DEFAULT_ASSETS_RE, DEFAULT_CONFIG_FILES, + DEFAULT_EXTENSIONS, + DEFAULT_MAIN_FIELDS, ENV_ENTRY } from './constants' import type { InternalResolveOptions, ResolveOptions } from './plugins/resolve' @@ -321,7 +323,7 @@ export type ResolvedConfig = Readonly< mainConfig: ResolvedConfig | null isProduction: boolean env: Record - resolve: ResolveOptions & { + resolve: Required & { alias: Alias[] } plugins: readonly Plugin[] @@ -474,7 +476,11 @@ export async function resolveConfig( ) const resolveOptions: ResolvedConfig['resolve'] = { - ...config.resolve, + mainFields: config.resolve?.mainFields ?? DEFAULT_MAIN_FIELDS, + conditions: config.resolve?.conditions ?? [], + extensions: config.resolve?.extensions ?? DEFAULT_EXTENSIONS, + dedupe: config.resolve?.dedupe ?? [], + preserveSymlinks: config.resolve?.preserveSymlinks ?? false, alias: resolvedAlias } @@ -581,8 +587,8 @@ export async function resolveConfig( const server = resolveServerOptions(resolvedRoot, config.server, logger) const ssr = resolveSSROptions( config.ssr, - config.legacy?.buildSsrCjsExternalHeuristics, - config.resolve?.preserveSymlinks + resolveOptions.preserveSymlinks, + config.legacy?.buildSsrCjsExternalHeuristics ) const middlewareMode = config?.server?.middlewareMode @@ -649,7 +655,7 @@ export async function resolveConfig( disabled: 'build', ...optimizeDeps, esbuildOptions: { - preserveSymlinks: config.resolve?.preserveSymlinks, + preserveSymlinks: resolveOptions.preserveSymlinks, ...optimizeDeps.esbuildOptions } }, diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 97b032b2833e11..66b798e5df948f 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -68,7 +68,7 @@ export interface ResolveOptions { preserveSymlinks?: boolean } -export interface InternalResolveOptions extends ResolveOptions { +export interface InternalResolveOptions extends Required { root: string isBuild: boolean isProduction: boolean @@ -84,7 +84,6 @@ export interface InternalResolveOptions extends ResolveOptions { tryPrefix?: string skipPackageJson?: boolean preferRelative?: boolean - preserveSymlinks?: boolean isRequire?: boolean // #3040 // when the importer is a ts module, @@ -450,7 +449,7 @@ function tryFsResolve( return res } - for (const ext of options.extensions || DEFAULT_EXTENSIONS) { + for (const ext of options.extensions) { if ( postfix && (res = tryResolveFile( @@ -926,7 +925,7 @@ export function resolvePackageEntry( } if (!entryPoint || entryPoint.endsWith('.mjs')) { - for (const field of options.mainFields || DEFAULT_MAIN_FIELDS) { + for (const field of options.mainFields) { if (typeof data[field] === 'string') { entryPoint = data[field] break @@ -944,8 +943,8 @@ export function resolvePackageEntry( for (let entry of entryPoints) { // make sure we don't get scripts when looking for sass if ( - options.mainFields?.[0] === 'sass' && - !options.extensions?.includes(path.extname(entry)) + options.mainFields[0] === 'sass' && + !options.extensions.includes(path.extname(entry)) ) { entry = '' options.skipPackageJson = true @@ -994,7 +993,7 @@ function resolveExports( if (!options.isRequire) { conditions.push('module') } - if (options.conditions) { + if (options.conditions.length > 0) { conditions.push(...options.conditions) } diff --git a/packages/vite/src/node/plugins/ssrRequireHook.ts b/packages/vite/src/node/plugins/ssrRequireHook.ts index d1173b211ff836..e02f9b48e6b6c7 100644 --- a/packages/vite/src/node/plugins/ssrRequireHook.ts +++ b/packages/vite/src/node/plugins/ssrRequireHook.ts @@ -13,7 +13,7 @@ export function ssrRequireHookPlugin(config: ResolvedConfig): Plugin | null { if ( config.command !== 'build' || !config.build.ssr || - !config.resolve.dedupe?.length || + !config.resolve.dedupe.length || config.ssr?.noExternal === true || config.ssr?.format !== 'cjs' || isBuildOutputEsm(config) diff --git a/packages/vite/src/node/ssr/index.ts b/packages/vite/src/node/ssr/index.ts index 7d2e4724f98b44..d23e78b18cae5f 100644 --- a/packages/vite/src/node/ssr/index.ts +++ b/packages/vite/src/node/ssr/index.ts @@ -41,8 +41,8 @@ export interface ResolvedSSROptions extends SSROptions { export function resolveSSROptions( ssr: SSROptions | undefined, - buildSsrCjsExternalHeuristics?: boolean, - preserveSymlinks?: boolean + preserveSymlinks: boolean, + buildSsrCjsExternalHeuristics?: boolean ): ResolvedSSROptions { ssr ??= {} const optimizeDeps = ssr.optimizeDeps ?? {} diff --git a/packages/vite/src/node/ssr/ssrExternal.ts b/packages/vite/src/node/ssr/ssrExternal.ts index d73d17c1d7c3c8..7c48c84db48743 100644 --- a/packages/vite/src/node/ssr/ssrExternal.ts +++ b/packages/vite/src/node/ssr/ssrExternal.ts @@ -1,7 +1,7 @@ import fs from 'node:fs' import path from 'node:path' import { createRequire } from 'node:module' -import type { InternalResolveOptions } from '../plugins/resolve' +import type { InternalResolveOptions, ResolveOptions } from '../plugins/resolve' import { tryNodeResolve } from '../plugins/resolve' import { bareImportRE, @@ -53,7 +53,7 @@ export function cjsSsrResolveExternals( cjsSsrCollectExternals( config.root, - config.resolve.preserveSymlinks, + config.resolve, ssrExternals, seen, config.logger @@ -116,8 +116,8 @@ export function createIsConfiguredAsSsrExternal( createFilter(undefined, noExternal, { resolve: false }) const resolveOptions: InternalResolveOptions = { + ...config.resolve, root, - preserveSymlinks: config.resolve.preserveSymlinks, isProduction: false, isBuild: true } @@ -211,7 +211,7 @@ function createIsSsrExternal( // is used reverting to the Vite 2.9 SSR externalization heuristics function cjsSsrCollectExternals( root: string, - preserveSymlinks: boolean | undefined, + resolveOptions: Required, ssrExternals: Set, seen: Set, logger: Logger @@ -227,9 +227,9 @@ function cjsSsrCollectExternals( ...rootPkg.dependencies } - const resolveOptions: InternalResolveOptions = { + const internalResolveOptions: InternalResolveOptions = { + ...resolveOptions, root, - preserveSymlinks, isProduction: false, isBuild: true } @@ -247,7 +247,7 @@ function cjsSsrCollectExternals( esmEntry = tryNodeResolve( id, undefined, - resolveOptions, + internalResolveOptions, true, // we set `targetWeb` to `true` to get the ESM entry undefined, true @@ -314,13 +314,7 @@ function cjsSsrCollectExternals( } for (const depRoot of depsToTrace) { - cjsSsrCollectExternals( - depRoot, - preserveSymlinks, - ssrExternals, - seen, - logger - ) + cjsSsrCollectExternals(depRoot, resolveOptions, ssrExternals, seen, logger) } } diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 8f61125c134d8c..9c8ef42ce5de38 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -119,13 +119,14 @@ async function instantiateModule( // CommonJS modules are preferred. We want to avoid ESM->ESM imports // whenever possible, because `hookNodeResolve` can't intercept them. const resolveOptions: InternalResolveOptions = { - dedupe, + mainFields: ['main'], + conditions: [], extensions: ['.js', '.cjs', '.json'], + dedupe, + preserveSymlinks, isBuild: true, isProduction, isRequire: true, - mainFields: ['main'], - preserveSymlinks, root } From 55918d7b88c87fb08d728e589ef4e5acd848a516 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 11 Sep 2022 19:26:28 +0900 Subject: [PATCH 2/4] fix: respect mainFields when resolving browser/module field --- packages/vite/src/node/plugins/resolve.ts | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 66b798e5df948f..85f954293154d3 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -236,6 +236,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { if ( targetWeb && + options.mainFields.includes('browser') && (res = tryResolveBrowserMapping(fsPath, importer, options, true)) ) { return res @@ -306,6 +307,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { if ( targetWeb && + options.mainFields.includes('browser') && (res = tryResolveBrowserMapping( id, importer, @@ -884,7 +886,11 @@ export function resolvePackageEntry( // 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 && (!entryPoint || entryPoint.endsWith('.mjs'))) { + if ( + targetWeb && + options.mainFields.includes('browser') && + (!entryPoint || entryPoint.endsWith('.mjs')) + ) { // check browser field // https://github.com/defunctzombie/package-browser-field-spec const browserEntry = @@ -895,6 +901,7 @@ export function resolvePackageEntry( // check if the package also has a "module" field. if ( !options.isRequire && + options.mainFields.includes('module') && typeof data.module === 'string' && data.module !== browserEntry ) { @@ -926,6 +933,7 @@ export function resolvePackageEntry( if (!entryPoint || entryPoint.endsWith('.mjs')) { for (const field of options.mainFields) { + if (field === 'browser') continue // already checked above if (typeof data[field] === 'string') { entryPoint = data[field] break @@ -952,7 +960,11 @@ export function resolvePackageEntry( // resolve object browser field in package.json const { browser: browserField } = data - if (targetWeb && isObject(browserField)) { + if ( + targetWeb && + options.mainFields.includes('browser') && + isObject(browserField) + ) { entry = mapWithBrowserField(entry, browserField) || entry } @@ -1045,7 +1057,11 @@ function resolveDeepImport( `${path.join(dir, 'package.json')}.` ) } - } else if (targetWeb && isObject(browserField)) { + } else if ( + targetWeb && + options.mainFields.includes('browser') && + isObject(browserField) + ) { // resolve without postfix (see #7098) const { file, postfix } = splitFileAndPostfix(relativeId) const mapped = mapWithBrowserField(file, browserField) From 50ecd526a9f715ced6bfbeb3d989b68f123172cb Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 11 Sep 2022 21:53:16 +0900 Subject: [PATCH 3/4] fix: use `'!browser'` --- docs/config/shared-options.md | 4 ++++ packages/vite/src/node/plugins/resolve.ts | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/docs/config/shared-options.md b/docs/config/shared-options.md index a51350d05c462b..c8d1b63c074c97 100644 --- a/docs/config/shared-options.md +++ b/docs/config/shared-options.md @@ -158,6 +158,10 @@ Export keys ending with "/" is deprecated by Node and may not work well. Please List of fields in `package.json` to try when resolving a package's entry point. Note this takes lower precedence than conditional exports resolved from the `exports` field: if an entry point is successfully resolved from `exports`, the main field will be ignored. +:::warning `browser` field +Currently Vite resolves `browser` field even if `browser` is not included in `resolve.mainFields`. In future, `resolve.mainFields`'s default value will be `['browser', 'module', 'jsnext:main', 'jsnext']`. But for now, you could disable resolving to `browser` field by including `'!browser'` in `resolve.mainFields`. +::: + ## resolve.extensions - **Type:** `string[]` diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 85f954293154d3..c4b80f7ffd9f1e 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -236,7 +236,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { if ( targetWeb && - options.mainFields.includes('browser') && + mainFieldsContainsBrowser(options.mainFields) && (res = tryResolveBrowserMapping(fsPath, importer, options, true)) ) { return res @@ -307,7 +307,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { if ( targetWeb && - options.mainFields.includes('browser') && + mainFieldsContainsBrowser(options.mainFields) && (res = tryResolveBrowserMapping( id, importer, @@ -396,6 +396,14 @@ export default new Proxy({}, { } } +/** + * TODO + * @deprecated In Vite 4, this should be `mainFields.includes('browser')` + */ +function mainFieldsContainsBrowser(mainFields: string[]) { + return !mainFields.includes('!browser') +} + function splitFileAndPostfix(path: string) { let file = path let postfix = '' @@ -888,7 +896,7 @@ export function resolvePackageEntry( // fields are present, prioritize those instead. if ( targetWeb && - options.mainFields.includes('browser') && + mainFieldsContainsBrowser(options.mainFields) && (!entryPoint || entryPoint.endsWith('.mjs')) ) { // check browser field @@ -962,7 +970,7 @@ export function resolvePackageEntry( const { browser: browserField } = data if ( targetWeb && - options.mainFields.includes('browser') && + mainFieldsContainsBrowser(options.mainFields) && isObject(browserField) ) { entry = mapWithBrowserField(entry, browserField) || entry @@ -1059,7 +1067,7 @@ function resolveDeepImport( } } else if ( targetWeb && - options.mainFields.includes('browser') && + mainFieldsContainsBrowser(options.mainFields) && isObject(browserField) ) { // resolve without postfix (see #7098) From a31f641017c7cde963892794666b3a73b8dc5e8e Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Mon, 12 Sep 2022 19:46:47 +0900 Subject: [PATCH 4/4] refactor: `resolve.browserField` --- docs/config/shared-options.md | 12 +++++-- packages/vite/src/node/config.ts | 1 + packages/vite/src/node/plugins/resolve.ts | 31 ++++++------------- packages/vite/src/node/ssr/ssrModuleLoader.ts | 1 + 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/docs/config/shared-options.md b/docs/config/shared-options.md index c8d1b63c074c97..0ebddfd988bb04 100644 --- a/docs/config/shared-options.md +++ b/docs/config/shared-options.md @@ -158,9 +158,15 @@ Export keys ending with "/" is deprecated by Node and may not work well. Please List of fields in `package.json` to try when resolving a package's entry point. Note this takes lower precedence than conditional exports resolved from the `exports` field: if an entry point is successfully resolved from `exports`, the main field will be ignored. -:::warning `browser` field -Currently Vite resolves `browser` field even if `browser` is not included in `resolve.mainFields`. In future, `resolve.mainFields`'s default value will be `['browser', 'module', 'jsnext:main', 'jsnext']`. But for now, you could disable resolving to `browser` field by including `'!browser'` in `resolve.mainFields`. -::: +## resolve.browserField + +- **Type:** `boolean` +- **Default:** `true` +- **Deprecated** + +Whether to enable resolving to `browser` field. + +In future, `resolve.mainFields`'s default value will be `['browser', 'module', 'jsnext:main', 'jsnext']` and this option will be removed. ## resolve.extensions diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 6e02df35663c96..46ee9476677c2d 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -477,6 +477,7 @@ export async function resolveConfig( const resolveOptions: ResolvedConfig['resolve'] = { mainFields: config.resolve?.mainFields ?? DEFAULT_MAIN_FIELDS, + browserField: config.resolve?.browserField ?? true, conditions: config.resolve?.conditions ?? [], extensions: config.resolve?.extensions ?? DEFAULT_EXTENSIONS, dedupe: config.resolve?.dedupe ?? [], diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index c4b80f7ffd9f1e..01a7e478fa4998 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -62,6 +62,11 @@ const debug = createDebugger('vite:resolve-details', { export interface ResolveOptions { mainFields?: string[] + /** + * @deprecated In future, `mainFields` should be used instead. + * @default true + */ + browserField?: boolean conditions?: string[] extensions?: string[] dedupe?: string[] @@ -236,7 +241,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { if ( targetWeb && - mainFieldsContainsBrowser(options.mainFields) && + options.browserField && (res = tryResolveBrowserMapping(fsPath, importer, options, true)) ) { return res @@ -307,7 +312,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { if ( targetWeb && - mainFieldsContainsBrowser(options.mainFields) && + options.browserField && (res = tryResolveBrowserMapping( id, importer, @@ -396,14 +401,6 @@ export default new Proxy({}, { } } -/** - * TODO - * @deprecated In Vite 4, this should be `mainFields.includes('browser')` - */ -function mainFieldsContainsBrowser(mainFields: string[]) { - return !mainFields.includes('!browser') -} - function splitFileAndPostfix(path: string) { let file = path let postfix = '' @@ -896,7 +893,7 @@ export function resolvePackageEntry( // fields are present, prioritize those instead. if ( targetWeb && - mainFieldsContainsBrowser(options.mainFields) && + options.browserField && (!entryPoint || entryPoint.endsWith('.mjs')) ) { // check browser field @@ -968,11 +965,7 @@ export function resolvePackageEntry( // resolve object browser field in package.json const { browser: browserField } = data - if ( - targetWeb && - mainFieldsContainsBrowser(options.mainFields) && - isObject(browserField) - ) { + if (targetWeb && options.browserField && isObject(browserField)) { entry = mapWithBrowserField(entry, browserField) || entry } @@ -1065,11 +1058,7 @@ function resolveDeepImport( `${path.join(dir, 'package.json')}.` ) } - } else if ( - targetWeb && - mainFieldsContainsBrowser(options.mainFields) && - isObject(browserField) - ) { + } else if (targetWeb && options.browserField && isObject(browserField)) { // resolve without postfix (see #7098) const { file, postfix } = splitFileAndPostfix(relativeId) const mapped = mapWithBrowserField(file, browserField) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 9c8ef42ce5de38..6daf1a6123c804 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -120,6 +120,7 @@ async function instantiateModule( // whenever possible, because `hookNodeResolve` can't intercept them. const resolveOptions: InternalResolveOptions = { mainFields: ['main'], + browserField: true, conditions: [], extensions: ['.js', '.cjs', '.json'], dedupe,