Skip to content

Commit

Permalink
fix: exclude dependency of optimized dependency (fix: 5410) (#5411)
Browse files Browse the repository at this point in the history
Co-authored-by: Artem Khodyush <pakafpub@gmail.com>
Co-authored-by: Shinigami <chrissi92@hotmail.de>
  • Loading branch information
3 people committed Oct 27, 2021
1 parent baba1f9 commit ebd4027
Show file tree
Hide file tree
Showing 15 changed files with 111 additions and 8 deletions.
Expand Up @@ -7,4 +7,5 @@ test('handle nested package', async () => {
expect(await page.textContent('.side-c')).toBe(c)
expect(await page.textContent('.d')).toBe('D@1.0.0')
expect(await page.textContent('.nested-d')).toBe('D-nested@1.0.0')
expect(await page.textContent('.nested-e')).toBe('1')
})
6 changes: 6 additions & 0 deletions packages/playground/nested-deps/index.html
Expand Up @@ -19,12 +19,16 @@ <h2>direct dependency D</h2>
<h2>nested dependency nested-D (dep of D)</h2>
<pre class="nested-d"></pre>

<h2>exclude dependency of pre-bundled dependency</h2>
<div>nested module instance count: <span class="nested-e"></span></div>

<script type="module">
import A from 'test-package-a'
import B, { A as nestedA } from 'test-package-b'
import C from 'test-package-c'
import { C as sideC } from 'test-package-c/side'
import D, { nestedD } from 'test-package-d'
import { testExcluded } from 'test-package-e'

text('.a', A)
text('.b', B)
Expand All @@ -36,6 +40,8 @@ <h2>nested dependency nested-D (dep of D)</h2>
text('.d', D)
text('.nested-d', nestedD)

text('.nested-e', testExcluded())

function text(sel, text) {
document.querySelector(sel).textContent = text
}
Expand Down
3 changes: 2 additions & 1 deletion packages/playground/nested-deps/package.json
Expand Up @@ -12,6 +12,7 @@
"test-package-a": "link:./test-package-a",
"test-package-b": "link:./test-package-b",
"test-package-c": "link:./test-package-c",
"test-package-d": "link:./test-package-d"
"test-package-d": "link:./test-package-d",
"test-package-e": "link:./test-package-e"
}
}
2 changes: 2 additions & 0 deletions packages/playground/nested-deps/test-package-e/index.js
@@ -0,0 +1,2 @@
export { testIncluded } from 'test-package-e-included'
export { testExcluded } from 'test-package-e-excluded'
9 changes: 9 additions & 0 deletions packages/playground/nested-deps/test-package-e/package.json
@@ -0,0 +1,9 @@
{
"name": "test-package-e",
"version": "0.1.0",
"main": "index.js",
"dependencies": {
"test-package-e-excluded": "link:./test-package-e-excluded",
"test-package-e-included": "link:./test-package-e-included"
}
}
@@ -0,0 +1,11 @@
const key = '$$excludedDependencyInstanceCount'

if (!(key in window)) {
window[key] = 0
}

++window[key]

export function testExcluded() {
return window[key]
}
@@ -0,0 +1,5 @@
{
"name": "test-package-e-excluded",
"version": "0.1.0",
"main": "index.js"
}
@@ -0,0 +1,5 @@
import { testExcluded } from 'test-package-e-excluded'

export function testIncluded() {
return testExcluded()
}
@@ -0,0 +1,8 @@
{
"name": "test-package-e-included",
"version": "0.1.0",
"main": "index.js",
"dependencies": {
"test-package-e-excluded": "link:../test-package-e-excluded"
}
}
5 changes: 3 additions & 2 deletions packages/playground/nested-deps/vite.config.js
Expand Up @@ -8,8 +8,9 @@ module.exports = {
'test-package-b',
'test-package-c',
'test-package-c/side',
'test-package-d > test-package-d-nested'
'test-package-d > test-package-d-nested',
'test-package-e-included'
],
exclude: ['test-package-d']
exclude: ['test-package-d', 'test-package-e-excluded']
}
}
10 changes: 9 additions & 1 deletion packages/vite/src/node/optimizer/esbuildDepPlugin.ts
Expand Up @@ -6,7 +6,8 @@ import {
isRunningWithYarnPnp,
flattenId,
normalizePath,
isExternalUrl
isExternalUrl,
moduleListContains
} from '../utils'
import { browserExternalId } from '../plugins/resolve'
import { ExportsData } from '.'
Expand Down Expand Up @@ -99,6 +100,13 @@ export function esbuildDepPlugin(
build.onResolve(
{ filter: /^[\w@][^:]/ },
async ({ path: id, importer, kind }) => {
if (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
Expand Down
3 changes: 2 additions & 1 deletion packages/vite/src/node/optimizer/scan.ts
Expand Up @@ -14,6 +14,7 @@ import {
normalizePath,
isObject,
cleanUrl,
moduleListContains,
externalRE,
dataUrlRE,
multilineCommentsRE,
Expand Down Expand Up @@ -302,7 +303,7 @@ function esbuildScanPlugin(
filter: /^[\w@][^:]/
},
async ({ path: id, importer }) => {
if (exclude?.some((e) => e === id || id.startsWith(e + '/'))) {
if (moduleListContains(exclude, id)) {
return externalUnlessEntry({ path: id })
}
if (depImports[id]) {
Expand Down
25 changes: 22 additions & 3 deletions packages/vite/src/node/plugins/importAnalysis.ts
Expand Up @@ -19,7 +19,8 @@ import {
timeFrom,
normalizePath,
removeImportQuery,
unwrapId
unwrapId,
moduleListContains
} from '../utils'
import {
debugHmr,
Expand Down Expand Up @@ -179,13 +180,31 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
url = url.replace(base, '/')
}

const resolved = await this.resolve(url, importer)
let importerFile = importer
if (
moduleListContains(config.optimizeDeps?.exclude, url) &&
server._optimizeDepsMetadata
) {
// if the dependency encountered in the optimized file was excluded from the optimization
// the dependency needs to be resolved starting from the original source location of the optimized file
// because starting from node_modules/.vite will not find the dependency if it was not hoisted
// (that is, if it is under node_modules directory in the package source of the optimized file)
for (const optimizedModule of Object.values(
server._optimizeDepsMetadata.optimized
)) {
if (optimizedModule.file === importerModule.file) {
importerFile = optimizedModule.src
}
}
}

const resolved = await this.resolve(url, importerFile)

if (!resolved) {
this.error(
`Failed to resolve import "${url}" from "${path.relative(
process.cwd(),
importer
importerFile
)}". Does the file exist?`,
pos
)
Expand Down
7 changes: 7 additions & 0 deletions packages/vite/src/node/utils.ts
Expand Up @@ -43,6 +43,13 @@ export function isBuiltin(id: string): boolean {
return builtins.includes(id)
}

export function moduleListContains(
moduleList: string[] | undefined,
id: string
): boolean | undefined {
return moduleList?.some((m) => m === id || id.startsWith(m + '/'))
}

export const bareImportRE = /^[\w@](?!.*:\/\/)/
export const deepImportRE = /^([^@][^/]*)\/|^(@[^/]+\/[^/]+)\//

Expand Down
19 changes: 19 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 ebd4027

Please sign in to comment.