From 4a111aafd5b488514c84bc4a00f0e5640b0079fc Mon Sep 17 00:00:00 2001 From: Dominik G Date: Wed, 29 Nov 2023 11:52:39 +0100 Subject: [PATCH] fix(resolve): respect order of browser in mainFields when resolving (#15137) --- packages/vite/src/node/plugins/resolve.ts | 99 ++++++++++--------- .../resolve-mainfields-custom-first.spec.ts | 8 ++ playground/resolve/__tests__/resolve.spec.ts | 6 ++ .../index.browser.js | 1 + .../custom-browser-main-field/index.custom.js | 1 + .../custom-browser-main-field/index.js | 1 + .../custom-browser-main-field/package.json | 8 ++ playground/resolve/index.html | 6 ++ playground/resolve/package.json | 1 + .../vite.config-mainfields-custom-first.js | 6 ++ pnpm-lock.yaml | 5 + 11 files changed, 98 insertions(+), 44 deletions(-) create mode 100644 playground/resolve/__tests__/mainfields-custom-first/resolve-mainfields-custom-first.spec.ts create mode 100644 playground/resolve/custom-browser-main-field/index.browser.js create mode 100644 playground/resolve/custom-browser-main-field/index.custom.js create mode 100644 playground/resolve/custom-browser-main-field/index.js create mode 100644 playground/resolve/custom-browser-main-field/package.json create mode 100644 playground/resolve/vite.config-mainfields-custom-first.js diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index aeafad144182de..0c93628f6cfc5a 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -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 } @@ -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, diff --git a/playground/resolve/__tests__/mainfields-custom-first/resolve-mainfields-custom-first.spec.ts b/playground/resolve/__tests__/mainfields-custom-first/resolve-mainfields-custom-first.spec.ts new file mode 100644 index 00000000000000..c15f0f56d72375 --- /dev/null +++ b/playground/resolve/__tests__/mainfields-custom-first/resolve-mainfields-custom-first.spec.ts @@ -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', + ) +}) diff --git a/playground/resolve/__tests__/resolve.spec.ts b/playground/resolve/__tests__/resolve.spec.ts index d5067698e13713..45b87c23e4ab10 100644 --- a/playground/resolve/__tests__/resolve.spec.ts +++ b/playground/resolve/__tests__/resolve.spec.ts @@ -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]') }) diff --git a/playground/resolve/custom-browser-main-field/index.browser.js b/playground/resolve/custom-browser-main-field/index.browser.js new file mode 100644 index 00000000000000..a12f4a603c068d --- /dev/null +++ b/playground/resolve/custom-browser-main-field/index.browser.js @@ -0,0 +1 @@ +export const msg = 'resolved browser field' diff --git a/playground/resolve/custom-browser-main-field/index.custom.js b/playground/resolve/custom-browser-main-field/index.custom.js new file mode 100644 index 00000000000000..01ea529fad19b5 --- /dev/null +++ b/playground/resolve/custom-browser-main-field/index.custom.js @@ -0,0 +1 @@ +export const msg = 'resolved custom field' diff --git a/playground/resolve/custom-browser-main-field/index.js b/playground/resolve/custom-browser-main-field/index.js new file mode 100644 index 00000000000000..27f9fcfd43658b --- /dev/null +++ b/playground/resolve/custom-browser-main-field/index.js @@ -0,0 +1 @@ +export const msg = '[fail] resolved main field' diff --git a/playground/resolve/custom-browser-main-field/package.json b/playground/resolve/custom-browser-main-field/package.json new file mode 100644 index 00000000000000..0d372bc5eba9fc --- /dev/null +++ b/playground/resolve/custom-browser-main-field/package.json @@ -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" +} diff --git a/playground/resolve/index.html b/playground/resolve/index.html index 3d52ad6722e28c..89eced2e7149e9 100644 --- a/playground/resolve/index.html +++ b/playground/resolve/index.html @@ -158,6 +158,9 @@

resolve.extensions

resolve.mainFields

+

resolve.mainFields.custom-browser-main

+

+

resolve.conditions

@@ -341,6 +344,9 @@

resolve package that contains # in path

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) diff --git a/playground/resolve/package.json b/playground/resolve/package.json index c7807c500b5f54..dfbb6c65309c93 100644 --- a/playground/resolve/package.json +++ b/playground/resolve/package.json @@ -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", diff --git a/playground/resolve/vite.config-mainfields-custom-first.js b/playground/resolve/vite.config-mainfields-custom-first.js new file mode 100644 index 00000000000000..96279c461edd5b --- /dev/null +++ b/playground/resolve/vite.config-mainfields-custom-first.js @@ -0,0 +1,6 @@ +import config from './vite.config.js' +config.resolve.mainFields = [ + 'custom', + ...config.resolve.mainFields.filter((f) => f !== 'custom'), +] +export default config diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 41fea161948fe1..0c1b82ab2968a2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1071,6 +1071,9 @@ importers: '@vitejs/test-resolve-browser-module-field3': specifier: link:./browser-module-field3 version: link:browser-module-field3 + '@vitejs/test-resolve-custom-browser-main-field': + specifier: link:./custom-browser-main-field + version: link:custom-browser-main-field '@vitejs/test-resolve-custom-condition': specifier: link:./custom-condition version: link:custom-condition @@ -1140,6 +1143,8 @@ importers: playground/resolve/browser-module-field3: {} + playground/resolve/custom-browser-main-field: {} + playground/resolve/custom-condition: {} playground/resolve/custom-main-field: {}