Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: respect mainFields when resolving browser/module field (fixes #8659) #10071

Merged
merged 4 commits into from Sep 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/config/shared-options.md
Expand Up @@ -158,6 +158,16 @@ 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.

## 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

- **Type:** `string[]`
Expand Down
17 changes: 12 additions & 5 deletions packages/vite/src/node/config.ts
Expand Up @@ -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'
Expand Down Expand Up @@ -321,7 +323,7 @@ export type ResolvedConfig = Readonly<
mainConfig: ResolvedConfig | null
isProduction: boolean
env: Record<string, any>
resolve: ResolveOptions & {
resolve: Required<ResolveOptions> & {
alias: Alias[]
}
plugins: readonly Plugin[]
Expand Down Expand Up @@ -474,7 +476,12 @@ export async function resolveConfig(
)

const resolveOptions: ResolvedConfig['resolve'] = {
...config.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 ?? [],
preserveSymlinks: config.resolve?.preserveSymlinks ?? false,
alias: resolvedAlias
}

Expand Down Expand Up @@ -581,8 +588,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
Expand Down Expand Up @@ -649,7 +656,7 @@ export async function resolveConfig(
disabled: 'build',
...optimizeDeps,
esbuildOptions: {
preserveSymlinks: config.resolve?.preserveSymlinks,
preserveSymlinks: resolveOptions.preserveSymlinks,
...optimizeDeps.esbuildOptions
}
},
Expand Down
32 changes: 22 additions & 10 deletions packages/vite/src/node/plugins/resolve.ts
Expand Up @@ -62,13 +62,18 @@ 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[]
preserveSymlinks?: boolean
}

export interface InternalResolveOptions extends ResolveOptions {
export interface InternalResolveOptions extends Required<ResolveOptions> {
root: string
isBuild: boolean
isProduction: boolean
Expand All @@ -84,7 +89,6 @@ export interface InternalResolveOptions extends ResolveOptions {
tryPrefix?: string
skipPackageJson?: boolean
preferRelative?: boolean
preserveSymlinks?: boolean
isRequire?: boolean
// #3040
// when the importer is a ts module,
Expand Down Expand Up @@ -237,6 +241,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {

if (
targetWeb &&
options.browserField &&
(res = tryResolveBrowserMapping(fsPath, importer, options, true))
) {
return res
Expand Down Expand Up @@ -307,6 +312,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {

if (
targetWeb &&
options.browserField &&
(res = tryResolveBrowserMapping(
id,
importer,
Expand Down Expand Up @@ -450,7 +456,7 @@ function tryFsResolve(
return res
}

for (const ext of options.extensions || DEFAULT_EXTENSIONS) {
for (const ext of options.extensions) {
if (
postfix &&
(res = tryResolveFile(
Expand Down Expand Up @@ -885,7 +891,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.browserField &&
(!entryPoint || entryPoint.endsWith('.mjs'))
) {
// check browser field
// https://github.com/defunctzombie/package-browser-field-spec
const browserEntry =
Expand All @@ -896,6 +906,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
) {
Expand Down Expand Up @@ -926,7 +937,8 @@ export function resolvePackageEntry(
}

if (!entryPoint || entryPoint.endsWith('.mjs')) {
for (const field of options.mainFields || DEFAULT_MAIN_FIELDS) {
for (const field of options.mainFields) {
if (field === 'browser') continue // already checked above
if (typeof data[field] === 'string') {
entryPoint = data[field]
break
Expand All @@ -944,16 +956,16 @@ 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
}

// resolve object browser field in package.json
const { browser: browserField } = data
if (targetWeb && isObject(browserField)) {
if (targetWeb && options.browserField && isObject(browserField)) {
entry = mapWithBrowserField(entry, browserField) || entry
}

Expand Down Expand Up @@ -994,7 +1006,7 @@ function resolveExports(
if (!options.isRequire) {
conditions.push('module')
}
if (options.conditions) {
if (options.conditions.length > 0) {
conditions.push(...options.conditions)
}

Expand Down Expand Up @@ -1046,7 +1058,7 @@ function resolveDeepImport(
`${path.join(dir, 'package.json')}.`
)
}
} else if (targetWeb && isObject(browserField)) {
} else if (targetWeb && options.browserField && isObject(browserField)) {
// resolve without postfix (see #7098)
const { file, postfix } = splitFileAndPostfix(relativeId)
const mapped = mapWithBrowserField(file, browserField)
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/plugins/ssrRequireHook.ts
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/node/ssr/index.ts
Expand Up @@ -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 ?? {}
Expand Down
22 changes: 8 additions & 14 deletions 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,
Expand Down Expand Up @@ -53,7 +53,7 @@ export function cjsSsrResolveExternals(

cjsSsrCollectExternals(
config.root,
config.resolve.preserveSymlinks,
config.resolve,
ssrExternals,
seen,
config.logger
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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<ResolveOptions>,
ssrExternals: Set<string>,
seen: Set<string>,
logger: Logger
Expand All @@ -227,9 +227,9 @@ function cjsSsrCollectExternals(
...rootPkg.dependencies
}

const resolveOptions: InternalResolveOptions = {
const internalResolveOptions: InternalResolveOptions = {
...resolveOptions,
root,
preserveSymlinks,
isProduction: false,
isBuild: true
}
Expand All @@ -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
Expand Down Expand Up @@ -314,13 +314,7 @@ function cjsSsrCollectExternals(
}

for (const depRoot of depsToTrace) {
cjsSsrCollectExternals(
depRoot,
preserveSymlinks,
ssrExternals,
seen,
logger
)
cjsSsrCollectExternals(depRoot, resolveOptions, ssrExternals, seen, logger)
}
}

Expand Down
8 changes: 5 additions & 3 deletions packages/vite/src/node/ssr/ssrModuleLoader.ts
Expand Up @@ -119,13 +119,15 @@ 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'],
browserField: true,
conditions: [],
extensions: ['.js', '.cjs', '.json'],
dedupe,
preserveSymlinks,
isBuild: true,
isProduction,
isRequire: true,
mainFields: ['main'],
preserveSymlinks,
root
}

Expand Down