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(optimizer): browser field bare import #8709

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 26 additions & 26 deletions packages/vite/src/node/optimizer/esbuildDepPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ export function esbuildDepPlugin(
// explicit resolveDir - this is passed only during yarn pnp resolve for
// entries
if (resolveDir) {
_importer = normalizePath(path.join(resolveDir, '*'))
_importer = path.join(resolveDir, '*')
} else {
// map importer ids to file paths for correct resolution
_importer = importer in qualified ? qualified[importer] : importer
}
const resolver = kind.startsWith('require') ? _resolveRequire : _resolve
return resolver(id, _importer, undefined)
return resolver(id, normalizePath(_importer), undefined)
}

const resolveResult = (id: string, resolved: string) => {
Expand Down Expand Up @@ -149,35 +149,35 @@ export function esbuildDepPlugin(
}
}

build.onResolve(
{ filter: /^[\w@][^:]/ },
async ({ path: id, importer, kind }) => {
if (moduleListContains(config.optimizeDeps?.exclude, id)) {
return {
path: id,
external: true
}
build.onResolve({ filter: /./ }, async ({ path: id, importer, kind }) => {
if (
/^[\w@][^:]/.test(id) &&
moduleListContains(config.optimizeDeps?.exclude, id)
) {
return {
path: id,
external: true
}
}

// ensure esbuild uses our resolved entries
let entry: { path: string; namespace: string } | undefined
// if this is an entry, return entry namespace resolve result
if (!importer) {
if ((entry = resolveEntry(id))) return entry
// check if this is aliased to an entry - also return entry namespace
const aliased = await _resolve(id, undefined, true)
if (aliased && (entry = resolveEntry(aliased))) {
return entry
}
// ensure esbuild uses our resolved entries
let entry: { path: string; namespace: string } | undefined
// if this is an entry, return entry namespace resolve result
if (!importer) {
if ((entry = resolveEntry(id))) return entry
// check if this is aliased to an entry - also return entry namespace
const aliased = await _resolve(id, undefined, true)
if (aliased && (entry = resolveEntry(aliased))) {
return entry
}
}

// use vite's own resolver
const resolved = await resolve(id, importer, kind)
if (resolved) {
return resolveResult(id, resolved)
}
// use vite's own resolver
const resolved = await resolve(id, importer, kind)
if (resolved) {
return resolveResult(id, resolved)
}
)
})

// For entry files, we'll read it ourselves and construct a proxy module
// to retain the entry's raw id instead of file path so that esbuild
Expand Down
8 changes: 8 additions & 0 deletions playground/optimize-deps/__tests__/optimize-deps.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ test('cjs browser field (axios)', async () => {
expect(await page.textContent('.cjs-browser-field')).toBe('pong')
})

test('cjs browser field bare', async () => {
expect(await page.textContent('.cjs-browser-field-bare')).toBe('pong')
})

test('dep from linked dep (lodash-es)', async () => {
expect(await page.textContent('.deps-linked')).toBe('fooBarBaz')
})
Expand All @@ -77,6 +81,10 @@ test('import from dep with .notjs files', async () => {
expect(await page.textContent('.not-js')).toMatch(`[success]`)
})

test('Import from dependency which uses relative path which needs to be resolved by main field', async () => {
expect(await page.textContent('.relative-to-main')).toMatch(`[success]`)
})

test('dep with dynamic import', async () => {
expect(await page.textContent('.dep-with-dynamic-import')).toMatch(
`[success]`
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
foo: 'foo'
}
5 changes: 5 additions & 0 deletions playground/optimize-deps/dep-cjs-browser-field-bare/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use strict'

const internal = require('./internal')

module.exports = internal
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use strict'

const events = require('events')

module.exports = 'foo' in events ? 'pong' : ''
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "dep-cjs-browser-field-bare",
"private": true,
"version": "0.0.0",
"main": "index.js",
"browser": {
"events": "./events-shim.js"
}
}
1 change: 1 addition & 0 deletions playground/optimize-deps/dep-relative-to-main/entry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('./')
1 change: 1 addition & 0 deletions playground/optimize-deps/dep-relative-to-main/lib/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = '[success] imported from main'
6 changes: 6 additions & 0 deletions playground/optimize-deps/dep-relative-to-main/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "dep-relative-to-main",
"private": true,
"version": "1.0.0",
"main": "lib/main.js"
}
15 changes: 15 additions & 0 deletions playground/optimize-deps/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ <h2>Dedupe (dep in linked & optimized package)</h2>
<h2>CommonJS w/ browser field mapping (axios)</h2>
<div>This should show pong: <span class="cjs-browser-field"></span></div>

<h2>CommonJS w/ bare id browser field mapping</h2>
<div>This should show pong: <span class="cjs-browser-field-bare"></span></div>

<h2>Detecting linked src package and optimizing its deps (lodash-es)</h2>
<div>This should show fooBarBaz: <span class="deps-linked"></span></div>

Expand All @@ -50,6 +53,12 @@ <h2>Import from dependency with process.env.NODE_ENV</h2>
<h2>Import from dependency with .notjs files</h2>
<div class="not-js"></div>

<h2>
Import from dependency which uses relative path which needs to be resolved by
main field
</h2>
<div class="relative-to-main"></div>

<h2>Import from dependency with dynamic import</h2>
<div class="dep-with-dynamic-import"></div>

Expand Down Expand Up @@ -88,6 +97,9 @@ <h2>Flatten Id</h2>
// test dep detection in globbed files
const globbed = import.meta.glob('./glob/*.js', { eager: true })

import cjsBrowerFieldBare from 'dep-cjs-browser-field-bare'
text('.cjs-browser-field-bare', cjsBrowerFieldBare)

import { camelCase } from 'dep-linked'
text('.deps-linked', camelCase('foo-bar-baz'))

Expand All @@ -109,6 +121,9 @@ <h2>Flatten Id</h2>
import { notjsValue } from 'dep-not-js'
text('.not-js', notjsValue)

import foo from 'dep-relative-to-main/entry'
text('.relative-to-main', foo)

import { lazyFoo } from 'dep-with-dynamic-import'
lazyFoo().then((foo) => {
text('.dep-with-dynamic-import', foo)
Expand Down
2 changes: 2 additions & 0 deletions playground/optimize-deps/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"dependencies": {
"axios": "^0.27.2",
"clipboard": "^2.0.11",
"dep-cjs-browser-field-bare": "file:./dep-cjs-browser-field-bare",
"dep-cjs-compiled-from-cjs": "file:./dep-cjs-compiled-from-cjs",
"dep-cjs-compiled-from-esm": "file:./dep-cjs-compiled-from-esm",
"dep-cjs-with-assets": "file:./dep-cjs-with-assets",
Expand All @@ -19,6 +20,7 @@
"dep-linked-include": "link:./dep-linked-include",
"dep-node-env": "file:./dep-node-env",
"dep-not-js": "file:./dep-not-js",
"dep-relative-to-main": "file:./dep-relative-to-main",
"dep-with-builtin-module-cjs": "file:./dep-with-builtin-module-cjs",
"dep-with-builtin-module-esm": "file:./dep-with-builtin-module-esm",
"dep-with-dynamic-import": "file:./dep-with-dynamic-import",
Expand Down
22 changes: 22 additions & 0 deletions pnpm-lock.yaml

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