From 3613f31acda377e1abf27b20ceaa3465b6db136b Mon Sep 17 00:00:00 2001 From: csr632 <632882184@qq.com> Date: Sat, 10 Dec 2022 01:38:59 +0800 Subject: [PATCH 1/6] test: add test for resolve-optimized-dup-deps --- .../resolve-optimized-dup-deps.spec.ts | 7 ++++ .../resolve-optimized-dup-deps/index.html | 17 ++++++++ .../package-a/index.js | 6 +++ .../package-a/package.json | 9 +++++ .../package-b-v1/index.js | 3 ++ .../package-b-v1/package.json | 6 +++ .../package-b-v2/index.js | 3 ++ .../package-b-v2/package.json | 6 +++ .../resolve-optimized-dup-deps/package.json | 15 +++++++ .../resolve-optimized-dup-deps/vite.config.js | 8 ++++ pnpm-lock.yaml | 40 +++++++++++++++++++ 11 files changed, 120 insertions(+) create mode 100644 playground/resolve-optimized-dup-deps/__tests__/resolve-optimized-dup-deps.spec.ts create mode 100644 playground/resolve-optimized-dup-deps/index.html create mode 100644 playground/resolve-optimized-dup-deps/package-a/index.js create mode 100644 playground/resolve-optimized-dup-deps/package-a/package.json create mode 100644 playground/resolve-optimized-dup-deps/package-b-v1/index.js create mode 100644 playground/resolve-optimized-dup-deps/package-b-v1/package.json create mode 100644 playground/resolve-optimized-dup-deps/package-b-v2/index.js create mode 100644 playground/resolve-optimized-dup-deps/package-b-v2/package.json create mode 100644 playground/resolve-optimized-dup-deps/package.json create mode 100644 playground/resolve-optimized-dup-deps/vite.config.js diff --git a/playground/resolve-optimized-dup-deps/__tests__/resolve-optimized-dup-deps.spec.ts b/playground/resolve-optimized-dup-deps/__tests__/resolve-optimized-dup-deps.spec.ts new file mode 100644 index 00000000000000..8b24e3ac762305 --- /dev/null +++ b/playground/resolve-optimized-dup-deps/__tests__/resolve-optimized-dup-deps.spec.ts @@ -0,0 +1,7 @@ +import { expect, test } from 'vitest' +import { page } from '~utils' + +test.todo('resolve-optimized-dup-deps', async () => { + expect(await page.textContent('.a')).toBe('test-package-a:test-package-b-v2') + expect(await page.textContent('.b')).toBe('test-package-b-v1') +}) diff --git a/playground/resolve-optimized-dup-deps/index.html b/playground/resolve-optimized-dup-deps/index.html new file mode 100644 index 00000000000000..89b7bca36300e7 --- /dev/null +++ b/playground/resolve-optimized-dup-deps/index.html @@ -0,0 +1,17 @@ +

direct dependency A

+

+
+

direct dependency B

+

+
+
diff --git a/playground/resolve-optimized-dup-deps/package-a/index.js b/playground/resolve-optimized-dup-deps/package-a/index.js
new file mode 100644
index 00000000000000..52c82187d5ee47
--- /dev/null
+++ b/playground/resolve-optimized-dup-deps/package-a/index.js
@@ -0,0 +1,6 @@
+import b from '@vitejs/test-resolve-optimized-dup-deps-package-b'
+
+// should get test-package-a:test-package-b-v2
+const result = 'test-package-a:' + b
+
+export default result
diff --git a/playground/resolve-optimized-dup-deps/package-a/package.json b/playground/resolve-optimized-dup-deps/package-a/package.json
new file mode 100644
index 00000000000000..a2c0067d1573cb
--- /dev/null
+++ b/playground/resolve-optimized-dup-deps/package-a/package.json
@@ -0,0 +1,9 @@
+{
+  "name": "@vitejs/test-resolve-optimized-dup-deps-package-a",
+  "private": true,
+  "version": "1.0.0",
+  "main": "index.js",
+  "dependencies": {
+    "@vitejs/test-resolve-optimized-dup-deps-package-b": "file:../package-b-v2"
+  }
+}
diff --git a/playground/resolve-optimized-dup-deps/package-b-v1/index.js b/playground/resolve-optimized-dup-deps/package-b-v1/index.js
new file mode 100644
index 00000000000000..7d6c1ef0a992f1
--- /dev/null
+++ b/playground/resolve-optimized-dup-deps/package-b-v1/index.js
@@ -0,0 +1,3 @@
+// test-package-b-v1 is install and imported by user
+// it is written in cjs and should be optimized
+module.exports = 'test-package-b-v1'
diff --git a/playground/resolve-optimized-dup-deps/package-b-v1/package.json b/playground/resolve-optimized-dup-deps/package-b-v1/package.json
new file mode 100644
index 00000000000000..68b1c74f57f8e0
--- /dev/null
+++ b/playground/resolve-optimized-dup-deps/package-b-v1/package.json
@@ -0,0 +1,6 @@
+{
+  "name": "@vitejs/test-resolve-optimized-dup-deps-package-b",
+  "private": true,
+  "version": "1.0.0",
+  "main": "index.js"
+}
diff --git a/playground/resolve-optimized-dup-deps/package-b-v2/index.js b/playground/resolve-optimized-dup-deps/package-b-v2/index.js
new file mode 100644
index 00000000000000..204404e745db09
--- /dev/null
+++ b/playground/resolve-optimized-dup-deps/package-b-v2/index.js
@@ -0,0 +1,3 @@
+// test-package-b-v2 is install and imported by test-package-a
+// it is written in esm. it is not optimized
+export default 'test-package-b-v2'
diff --git a/playground/resolve-optimized-dup-deps/package-b-v2/package.json b/playground/resolve-optimized-dup-deps/package-b-v2/package.json
new file mode 100644
index 00000000000000..ba9c84ea4538d6
--- /dev/null
+++ b/playground/resolve-optimized-dup-deps/package-b-v2/package.json
@@ -0,0 +1,6 @@
+{
+  "name": "@vitejs/test-resolve-optimized-dup-deps-package-b",
+  "private": true,
+  "version": "2.0.0",
+  "main": "index.js"
+}
diff --git a/playground/resolve-optimized-dup-deps/package.json b/playground/resolve-optimized-dup-deps/package.json
new file mode 100644
index 00000000000000..71267db3985ddd
--- /dev/null
+++ b/playground/resolve-optimized-dup-deps/package.json
@@ -0,0 +1,15 @@
+{
+  "name": "@vitejs/test-resolve-optimized-dup-deps",
+  "private": true,
+  "version": "0.0.0",
+  "scripts": {
+    "dev": "vite",
+    "build": "vite build",
+    "debug": "node --inspect-brk ../../packages/vite/bin/vite",
+    "preview": "vite preview"
+  },
+  "dependencies": {
+    "@vitejs/test-resolve-optimized-dup-deps-package-a": "file:./package-a",
+    "@vitejs/test-resolve-optimized-dup-deps-package-b": "file:./package-b-v1"
+  }
+}
diff --git a/playground/resolve-optimized-dup-deps/vite.config.js b/playground/resolve-optimized-dup-deps/vite.config.js
new file mode 100644
index 00000000000000..2d6290f12b9098
--- /dev/null
+++ b/playground/resolve-optimized-dup-deps/vite.config.js
@@ -0,0 +1,8 @@
+/**
+ * @type {import('vite').UserConfig}
+ */
+module.exports = {
+  optimizeDeps: {
+    exclude: ['@vitejs/test-resolve-optimized-dup-deps-package-a'],
+  },
+}
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index 76d89d191360a5..a0980a333b0742 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -826,6 +826,26 @@ importers:
   playground/resolve-linked:
     specifiers: {}
 
+  playground/resolve-optimized-dup-deps:
+    specifiers:
+      '@vitejs/test-resolve-optimized-dup-deps-package-a': file:./package-a
+      '@vitejs/test-resolve-optimized-dup-deps-package-b': file:./package-b-v1
+    dependencies:
+      '@vitejs/test-resolve-optimized-dup-deps-package-a': file:playground/resolve-optimized-dup-deps/package-a
+      '@vitejs/test-resolve-optimized-dup-deps-package-b': file:playground/resolve-optimized-dup-deps/package-b-v1
+
+  playground/resolve-optimized-dup-deps/package-a:
+    specifiers:
+      '@vitejs/test-resolve-optimized-dup-deps-package-b': file:../package-b-v2
+    dependencies:
+      '@vitejs/test-resolve-optimized-dup-deps-package-b': file:playground/resolve-optimized-dup-deps/package-b-v2
+
+  playground/resolve-optimized-dup-deps/package-b-v1:
+    specifiers: {}
+
+  playground/resolve-optimized-dup-deps/package-b-v2:
+    specifiers: {}
+
   playground/resolve/browser-field:
     specifiers: {}
 
@@ -8667,6 +8687,26 @@ packages:
       dep-a: file:playground/preload/dep-a
     dev: true
 
+  file:playground/resolve-optimized-dup-deps/package-a:
+    resolution: {directory: playground/resolve-optimized-dup-deps/package-a, type: directory}
+    name: '@vitejs/test-resolve-optimized-dup-deps-package-a'
+    version: 1.0.0
+    dependencies:
+      '@vitejs/test-resolve-optimized-dup-deps-package-b': file:playground/resolve-optimized-dup-deps/package-b-v2
+    dev: false
+
+  file:playground/resolve-optimized-dup-deps/package-b-v1:
+    resolution: {directory: playground/resolve-optimized-dup-deps/package-b-v1, type: directory}
+    name: '@vitejs/test-resolve-optimized-dup-deps-package-b'
+    version: 1.0.0
+    dev: false
+
+  file:playground/resolve-optimized-dup-deps/package-b-v2:
+    resolution: {directory: playground/resolve-optimized-dup-deps/package-b-v2, type: directory}
+    name: '@vitejs/test-resolve-optimized-dup-deps-package-b'
+    version: 2.0.0
+    dev: false
+
   file:playground/ssr-deps/css-lib:
     resolution: {directory: playground/ssr-deps/css-lib, type: directory}
     name: '@vitejs/test-css-lib'

From bdcc66af7cad69a1d1986c791b7f6a982d5c1efd Mon Sep 17 00:00:00 2001
From: csr632 <632882184@qq.com>
Date: Sat, 10 Dec 2022 14:53:10 +0800
Subject: [PATCH 2/6] fix: resolve to an optimizedDep only if it is what the
 importer wants

---
 packages/vite/src/node/plugins/resolve.ts     | 34 +++++++++----------
 .../resolve-optimized-dup-deps.spec.ts        |  2 +-
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts
index 2c45771c46e101..197194bd140bd2 100644
--- a/packages/vite/src/node/plugins/resolve.ts
+++ b/packages/vite/src/node/plugins/resolve.ts
@@ -872,18 +872,27 @@ export async function tryOptimizedResolve(
   // We should be able to remove this in the future
   await depsOptimizer.scanProcessing
 
+  // resolvedSrc is what the importer wants
+  // we can return a optimizedDep only if optimizedDep.src === resolvedSrc
+  // https://github.com/vitejs/vite/pull/11290
+  let resolvedSrc: string | undefined
+  try {
+    if (importer)
+      // this may throw errors if unable to resolve, e.g. aliased id
+      resolvedSrc = normalizePath(resolveFrom(id, path.dirname(importer)))
+  } catch {
+    // this is best-effort only so swallow errors
+  }
+  if (!resolvedSrc) return
+
   const metadata = depsOptimizer.metadata
 
   const depInfo = optimizedDepInfoFromId(metadata, id)
-  if (depInfo) {
+  // check if the found optimizedDep comes from the resolvedSrc
+  if (depInfo && depInfo.src === resolvedSrc) {
     return depsOptimizer.getOptimizedDepId(depInfo)
   }
 
-  if (!importer) return
-
-  // further check if id is imported by nested dependency
-  let resolvedSrc: string | undefined
-
   for (const optimizedData of metadata.depInfoList) {
     if (!optimizedData.src) continue // Ignore chunks
 
@@ -894,18 +903,7 @@ export async function tryOptimizedResolve(
     // this narrows the need to do a full resolve
     if (!pkgPath.endsWith(id)) continue
 
-    // lazily initialize resolvedSrc
-    if (resolvedSrc == null) {
-      try {
-        // this may throw errors if unable to resolve, e.g. aliased id
-        resolvedSrc = normalizePath(resolveFrom(id, path.dirname(importer)))
-      } catch {
-        // this is best-effort only so swallow errors
-        break
-      }
-    }
-
-    // match by src to correctly identify if id belongs to nested dependency
+    // check if the found optimizedDep comes from the resolvedSrc
     if (optimizedData.src === resolvedSrc) {
       return depsOptimizer.getOptimizedDepId(optimizedData)
     }
diff --git a/playground/resolve-optimized-dup-deps/__tests__/resolve-optimized-dup-deps.spec.ts b/playground/resolve-optimized-dup-deps/__tests__/resolve-optimized-dup-deps.spec.ts
index 8b24e3ac762305..e2c1507ecdb2ff 100644
--- a/playground/resolve-optimized-dup-deps/__tests__/resolve-optimized-dup-deps.spec.ts
+++ b/playground/resolve-optimized-dup-deps/__tests__/resolve-optimized-dup-deps.spec.ts
@@ -1,7 +1,7 @@
 import { expect, test } from 'vitest'
 import { page } from '~utils'
 
-test.todo('resolve-optimized-dup-deps', async () => {
+test('resolve-optimized-dup-deps', async () => {
   expect(await page.textContent('.a')).toBe('test-package-a:test-package-b-v2')
   expect(await page.textContent('.b')).toBe('test-package-b-v1')
 })

From 206caee8bd28128a77e94701f472e445a074e29d Mon Sep 17 00:00:00 2001
From: csr632 <632882184@qq.com>
Date: Sat, 10 Dec 2022 21:03:41 +0800
Subject: [PATCH 3/6] fix: handle no importer case

---
 packages/vite/src/node/plugins/resolve.ts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts
index 197194bd140bd2..55057a6cffe50a 100644
--- a/packages/vite/src/node/plugins/resolve.ts
+++ b/packages/vite/src/node/plugins/resolve.ts
@@ -876,10 +876,10 @@ export async function tryOptimizedResolve(
   // we can return a optimizedDep only if optimizedDep.src === resolvedSrc
   // https://github.com/vitejs/vite/pull/11290
   let resolvedSrc: string | undefined
+  const basedir = importer ? path.dirname(importer) : process.cwd()
   try {
-    if (importer)
-      // this may throw errors if unable to resolve, e.g. aliased id
-      resolvedSrc = normalizePath(resolveFrom(id, path.dirname(importer)))
+    // this may throw errors if unable to resolve, e.g. aliased id
+    resolvedSrc = normalizePath(resolveFrom(id, basedir))
   } catch {
     // this is best-effort only so swallow errors
   }

From 647de041073dcb6470d40b66e066c80738759def Mon Sep 17 00:00:00 2001
From: csr632 <632882184@qq.com>
Date: Sun, 11 Dec 2022 00:48:13 +0800
Subject: [PATCH 4/6] fix: simplify !importer case

---
 packages/vite/src/node/plugins/resolve.ts | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts
index 55057a6cffe50a..3f445bc8d596cc 100644
--- a/packages/vite/src/node/plugins/resolve.ts
+++ b/packages/vite/src/node/plugins/resolve.ts
@@ -872,22 +872,29 @@ export async function tryOptimizedResolve(
   // We should be able to remove this in the future
   await depsOptimizer.scanProcessing
 
+  const metadata = depsOptimizer.metadata
+
+  const depInfo = optimizedDepInfoFromId(metadata, id)
+  if (!importer) {
+    // no importer. try our best to find an optimized dep
+    if (depInfo) {
+      return depsOptimizer.getOptimizedDepId(depInfo)
+    }
+    return
+  }
+
   // resolvedSrc is what the importer wants
   // we can return a optimizedDep only if optimizedDep.src === resolvedSrc
   // https://github.com/vitejs/vite/pull/11290
   let resolvedSrc: string | undefined
-  const basedir = importer ? path.dirname(importer) : process.cwd()
   try {
     // this may throw errors if unable to resolve, e.g. aliased id
-    resolvedSrc = normalizePath(resolveFrom(id, basedir))
+    resolvedSrc = normalizePath(resolveFrom(id, path.dirname(importer)))
   } catch {
     // this is best-effort only so swallow errors
   }
   if (!resolvedSrc) return
 
-  const metadata = depsOptimizer.metadata
-
-  const depInfo = optimizedDepInfoFromId(metadata, id)
   // check if the found optimizedDep comes from the resolvedSrc
   if (depInfo && depInfo.src === resolvedSrc) {
     return depsOptimizer.getOptimizedDepId(depInfo)

From 1ff6fea5e464bfc743743f20e80487c605840ee9 Mon Sep 17 00:00:00 2001
From: csr632 <632882184@qq.com>
Date: Sun, 11 Dec 2022 18:14:09 +0800
Subject: [PATCH 5/6] fix: call resolveFrom lazily

---
 packages/vite/src/node/plugins/resolve.ts | 27 +++++++++++------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts
index 3f445bc8d596cc..0ca368177facc5 100644
--- a/packages/vite/src/node/plugins/resolve.ts
+++ b/packages/vite/src/node/plugins/resolve.ts
@@ -874,8 +874,8 @@ export async function tryOptimizedResolve(
 
   const metadata = depsOptimizer.metadata
 
-  const depInfo = optimizedDepInfoFromId(metadata, id)
   if (!importer) {
+    const depInfo = optimizedDepInfoFromId(metadata, id)
     // no importer. try our best to find an optimized dep
     if (depInfo) {
       return depsOptimizer.getOptimizedDepId(depInfo)
@@ -884,21 +884,9 @@ export async function tryOptimizedResolve(
   }
 
   // resolvedSrc is what the importer wants
-  // we can return a optimizedDep only if optimizedDep.src === resolvedSrc
+  // we can return an optimizedDep only if optimizedDep.src === resolvedSrc
   // https://github.com/vitejs/vite/pull/11290
   let resolvedSrc: string | undefined
-  try {
-    // this may throw errors if unable to resolve, e.g. aliased id
-    resolvedSrc = normalizePath(resolveFrom(id, path.dirname(importer)))
-  } catch {
-    // this is best-effort only so swallow errors
-  }
-  if (!resolvedSrc) return
-
-  // check if the found optimizedDep comes from the resolvedSrc
-  if (depInfo && depInfo.src === resolvedSrc) {
-    return depsOptimizer.getOptimizedDepId(depInfo)
-  }
 
   for (const optimizedData of metadata.depInfoList) {
     if (!optimizedData.src) continue // Ignore chunks
@@ -910,6 +898,17 @@ export async function tryOptimizedResolve(
     // this narrows the need to do a full resolve
     if (!pkgPath.endsWith(id)) continue
 
+    try {
+      // compute resolvedSrc lazily because resolveFrom is expensive
+      // this may throw errors if unable to resolve, e.g. aliased id
+      resolvedSrc = normalizePath(resolveFrom(id, path.dirname(importer)))
+    } catch {
+      // this is best-effort only so swallow errors
+    }
+    // if we can't compute resolvedSrc, return early
+    // no need to try again in next iteration
+    if (!resolvedSrc) return
+
     // check if the found optimizedDep comes from the resolvedSrc
     if (optimizedData.src === resolvedSrc) {
       return depsOptimizer.getOptimizedDepId(optimizedData)

From df9c54dbcc98df4217c4c87d80acc6bfa16410fd Mon Sep 17 00:00:00 2001
From: csr632 <632882184@qq.com>
Date: Mon, 12 Dec 2022 21:07:01 +0800
Subject: [PATCH 6/6] fix: simplify

---
 packages/vite/src/node/plugins/resolve.ts | 26 +++++++++++------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts
index 0ca368177facc5..cf4c91e6bfc636 100644
--- a/packages/vite/src/node/plugins/resolve.ts
+++ b/packages/vite/src/node/plugins/resolve.ts
@@ -875,17 +875,15 @@ export async function tryOptimizedResolve(
   const metadata = depsOptimizer.metadata
 
   if (!importer) {
-    const depInfo = optimizedDepInfoFromId(metadata, id)
     // no importer. try our best to find an optimized dep
+    const depInfo = optimizedDepInfoFromId(metadata, id)
     if (depInfo) {
       return depsOptimizer.getOptimizedDepId(depInfo)
     }
     return
   }
 
-  // resolvedSrc is what the importer wants
-  // we can return an optimizedDep only if optimizedDep.src === resolvedSrc
-  // https://github.com/vitejs/vite/pull/11290
+  // further check if id is imported by nested dependency
   let resolvedSrc: string | undefined
 
   for (const optimizedData of metadata.depInfoList) {
@@ -898,18 +896,18 @@ export async function tryOptimizedResolve(
     // this narrows the need to do a full resolve
     if (!pkgPath.endsWith(id)) continue
 
-    try {
-      // compute resolvedSrc lazily because resolveFrom is expensive
-      // this may throw errors if unable to resolve, e.g. aliased id
-      resolvedSrc = normalizePath(resolveFrom(id, path.dirname(importer)))
-    } catch {
-      // this is best-effort only so swallow errors
+    // lazily initialize resolvedSrc
+    if (resolvedSrc == null) {
+      try {
+        // this may throw errors if unable to resolve, e.g. aliased id
+        resolvedSrc = normalizePath(resolveFrom(id, path.dirname(importer)))
+      } catch {
+        // this is best-effort only so swallow errors
+        break
+      }
     }
-    // if we can't compute resolvedSrc, return early
-    // no need to try again in next iteration
-    if (!resolvedSrc) return
 
-    // check if the found optimizedDep comes from the resolvedSrc
+    // match by src to correctly identify if id belongs to nested dependency
     if (optimizedData.src === resolvedSrc) {
       return depsOptimizer.getOptimizedDepId(optimizedData)
     }