fix(optimizer): don't externalize transitive dep package name with asset extension (#18152)

Author: hi-ogawa

packages/vite/src/node/optimizer/esbuildDepPlugin.ts

@@ -1,6 +1,6 @@
 import path from 'node:path'
 import type { ImportKind, Plugin } from 'esbuild'
-import { KNOWN_ASSET_TYPES } from '../constants'
+import { JS_TYPES_RE, KNOWN_ASSET_TYPES } from '../constants'
 import type { PackageCache } from '../packages'
 import {
   escapeRegex,
@@ -157,15 +157,17 @@ export function esbuildDepPlugin(
 
           const resolved = await resolve(id, importer, kind)
           if (resolved) {
-            if (kind === 'require-call') {
-              // #16116 fix: Import the module.scss path, which is actually module.scss.js
-              if (resolved.endsWith('.js')) {
-                return {
-                  path: resolved,
-                  external: false,
-                }
+            // `resolved` can be javascript even when `id` matches `allExternalTypes`
+            // due to cjs resolution (e.g. require("./test.pdf") for "./test.pdf.js")
+            // or package name (e.g. import "some-package.pdf")
+            if (JS_TYPES_RE.test(resolved)) {
+              return {
+                path: resolved,
+                external: false,
               }
+            }
 
+            if (kind === 'require-call') {
               // here it is not set to `external: true` to convert `require` to `import`
               return {
                 path: resolved,

playground/optimize-deps/__tests__/optimize-deps.spec.ts

@@ -343,3 +343,12 @@ test('import the CommonJS external package that omits the js suffix', async () =
     page.textContent('.external-package-tsx-js'),
   ).toBe('tsx')
 })
+
+test('external package name with asset extension', async () => {
+  await expectWithRetry(() =>
+    page.textContent('.dep-with-asset-ext-no-dual-package'),
+  ).toBe('true')
+  await expectWithRetry(() =>
+    page.textContent('.dep-with-asset-ext-prebundled'),
+  ).toBe(String(isServe))
+})

playground/optimize-deps/dep-with-asset-ext/dep1/index.mjs

@@ -0,0 +1,3 @@
+export default { random: Math.random() }
+
+export const isPreBundled = import.meta.url.includes('/.vite/deps/')

playground/optimize-deps/dep-with-asset-ext/dep1/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "@vitejs/test-dep-with-asset-ext1.pdf",
+  "private": true,
+  "type": "module",
+  "exports": "./index.mjs"
+}

playground/optimize-deps/dep-with-asset-ext/dep2/index.js

@@ -0,0 +1 @@
+export { default } from '@vitejs/test-dep-with-asset-ext1.pdf'

playground/optimize-deps/dep-with-asset-ext/dep2/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "@vitejs/test-dep-with-asset-ext2.pdf",
+  "private": true,
+  "type": "module",
+  "exports": "./index.js",
+  "dependencies": {
+    "@vitejs/test-dep-with-asset-ext1.pdf": "file:../dep1"
+  }
+}

playground/optimize-deps/index.html

@@ -287,3 +287,15 @@ <h2>Import the CommonJS external package that omits the js suffix</h2>
   import loadSub from '@vitejs/test-dep-incompatible'
   loadSub() // should show an error that tells there's an incompatible dep
 </script>
+
+<h2>Pre-bundle transitive dependency 'some-package.pdf'</h2>
+<div>prebundled: <span class="dep-with-asset-ext-prebundled">???</span></div>
+<div>
+  no dual package: <span class="dep-with-asset-ext-no-dual-package">???</span>
+</div>
+<script type="module">
+  import original, { isPreBundled } from '@vitejs/test-dep-with-asset-ext1.pdf'
+  import reexport from '@vitejs/test-dep-with-asset-ext2.pdf'
+  text('.dep-with-asset-ext-prebundled', isPreBundled)
+  text('.dep-with-asset-ext-no-dual-package', original === reexport)
+</script>

playground/optimize-deps/package.json

@@ -29,6 +29,8 @@
     "@vitejs/test-dep-optimize-exports-with-root-glob": "file:./dep-optimize-exports-with-root-glob",
     "@vitejs/test-dep-optimize-with-glob": "file:./dep-optimize-with-glob",
     "@vitejs/test-dep-relative-to-main": "file:./dep-relative-to-main",
+    "@vitejs/test-dep-with-asset-ext1.pdf": "file:./dep-with-asset-ext/dep1",
+    "@vitejs/test-dep-with-asset-ext2.pdf": "file:./dep-with-asset-ext/dep2",
     "@vitejs/test-dep-with-builtin-module-cjs": "file:./dep-with-builtin-module-cjs",
     "@vitejs/test-dep-with-builtin-module-esm": "file:./dep-with-builtin-module-esm",
     "@vitejs/test-dep-with-dynamic-import": "file:./dep-with-dynamic-import",