Skip to content

Commit

Permalink
fix(resolve): respect order of browser in mainFields when resolving (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
dominikg authored Nov 29, 2023
1 parent 793cebd commit 4a111aa
Show file tree
Hide file tree
Showing 11 changed files with 98 additions and 44 deletions.
99 changes: 55 additions & 44 deletions packages/vite/src/node/plugins/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -982,53 +982,17 @@ export function resolvePackageEntry(
)
}

// handle edge case with browser and module field semantics
if (!entryPoint && targetWeb && options.mainFields.includes('browser')) {
// 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 (
!options.isRequire &&
options.mainFields.includes('module') &&
typeof data.module === 'string' &&
data.module !== browserEntry
) {
// if both are present, we may have a problem: some package points both
// to ESM, with "module" targeting Node.js, while some packages points
// "module" to browser ESM and "browser" to UMD/IIFE.
// the heuristics here is to actually read the browser entry when
// possible and check for hints of ESM. If it is not ESM, prefer "module"
// instead; Otherwise, assume it's ESM and use it.
const resolvedBrowserEntry = tryFsResolve(
path.join(dir, browserEntry),
options,
)
if (resolvedBrowserEntry) {
const content = fs.readFileSync(resolvedBrowserEntry, 'utf-8')
if (hasESMSyntax(content)) {
// likely ESM, prefer browser
entryPoint = browserEntry
} else {
// non-ESM, UMD or IIFE or CJS(!!! e.g. firebase 7.x), prefer module
entryPoint = data.module
}
}
} else {
entryPoint = browserEntry
}
}
}

// fallback to mainFields if still not resolved
if (!entryPoint) {
for (const field of options.mainFields) {
if (field === 'browser') continue // already checked above
if (typeof data[field] === 'string') {
if (field === 'browser') {
if (targetWeb) {
entryPoint = tryResolveBrowserEntry(dir, data, options)
if (entryPoint) {
break
}
}
} else if (typeof data[field] === 'string') {
entryPoint = data[field]
break
}
Expand Down Expand Up @@ -1257,6 +1221,53 @@ function tryResolveBrowserMapping(
}
}

function tryResolveBrowserEntry(
dir: string,
data: PackageData['data'],
options: InternalResolveOptions,
) {
// handle edge case with browser and module field semantics

// 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 (
!options.isRequire &&
options.mainFields.includes('module') &&
typeof data.module === 'string' &&
data.module !== browserEntry
) {
// if both are present, we may have a problem: some package points both
// to ESM, with "module" targeting Node.js, while some packages points
// "module" to browser ESM and "browser" to UMD/IIFE.
// the heuristics here is to actually read the browser entry when
// possible and check for hints of ESM. If it is not ESM, prefer "module"
// instead; Otherwise, assume it's ESM and use it.
const resolvedBrowserEntry = tryFsResolve(
path.join(dir, browserEntry),
options,
)
if (resolvedBrowserEntry) {
const content = fs.readFileSync(resolvedBrowserEntry, 'utf-8')
if (hasESMSyntax(content)) {
// likely ESM, prefer browser
return browserEntry
} else {
// non-ESM, UMD or IIFE or CJS(!!! e.g. firebase 7.x), prefer module
return data.module
}
}
} else {
return browserEntry
}
}
}

/**
* given a relative path in pkg dir,
* return a relative path in pkg dir,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { expect, test } from 'vitest'
import { page } from '~utils'

test('resolve.mainFields.custom-first', async () => {
expect(await page.textContent('.custom-browser-main-field')).toBe(
'resolved custom field',
)
})
6 changes: 6 additions & 0 deletions playground/resolve/__tests__/resolve.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,12 @@ test('resolve.mainFields', async () => {
expect(await page.textContent('.custom-main-fields')).toMatch('[success]')
})

test('resolve.mainFields.browser-first', async () => {
expect(await page.textContent('.custom-browser-main-field')).toBe(
'resolved browser field',
)
})

test('resolve.conditions', async () => {
expect(await page.textContent('.custom-condition')).toMatch('[success]')
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const msg = 'resolved browser field'
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const msg = 'resolved custom field'
1 change: 1 addition & 0 deletions playground/resolve/custom-browser-main-field/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const msg = '[fail] resolved main field'
8 changes: 8 additions & 0 deletions playground/resolve/custom-browser-main-field/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@vitejs/test-resolve-custom-browser-main-field",
"private": true,
"version": "1.0.0",
"main": "index.js",
"browser": "index.browser.js",
"custom": "index.custom.js"
}
6 changes: 6 additions & 0 deletions playground/resolve/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ <h2>resolve.extensions</h2>
<h2>resolve.mainFields</h2>
<p class="custom-main-fields"></p>

<h2>resolve.mainFields.custom-browser-main</h2>
<p class="custom-browser-main-field"></p>

<h2>resolve.conditions</h2>
<p class="custom-condition"></p>

Expand Down Expand Up @@ -341,6 +344,9 @@ <h2>resolve package that contains # in path</h2>
import { msg as customMainMsg } from '@vitejs/test-resolve-custom-main-field'
text('.custom-main-fields', customMainMsg)

import { msg as customBrowserMsg } from '@vitejs/test-resolve-custom-browser-main-field'
text('.custom-browser-main-field', customBrowserMsg)

import { msg as customConditionMsg } from '@vitejs/test-resolve-custom-condition'
text('.custom-condition', customConditionMsg)

Expand Down
1 change: 1 addition & 0 deletions playground/resolve/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"@vitejs/test-resolve-browser-module-field3": "link:./browser-module-field3",
"@vitejs/test-resolve-custom-condition": "link:./custom-condition",
"@vitejs/test-resolve-custom-main-field": "link:./custom-main-field",
"@vitejs/test-resolve-custom-browser-main-field": "link:./custom-browser-main-field",
"@vitejs/test-resolve-exports-and-nested-scope": "link:./exports-and-nested-scope",
"@vitejs/test-resolve-exports-env": "link:./exports-env",
"@vitejs/test-resolve-exports-from-root": "link:./exports-from-root",
Expand Down
6 changes: 6 additions & 0 deletions playground/resolve/vite.config-mainfields-custom-first.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import config from './vite.config.js'
config.resolve.mainFields = [
'custom',
...config.resolve.mainFields.filter((f) => f !== 'custom'),
]
export default config
5 changes: 5 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 4a111aa

Please sign in to comment.