fix(optimizer): external require-import conversion (fixes #2492, #3409, #5308)

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

@@ -79,21 +79,47 @@ export function esbuildDepPlugin(
   return {
     name: 'vite:dep-pre-bundle',
     setup(build) {
+      const externalWithConversionNamespace =
+        'vite:dep-pre-bundle:external-conversion'
+      const convertedExternalPrefix = 'vite-dep-pre-bundle-external:'
+
       // externalize assets and commonly known non-js file types
+      // See #8459 for more details about this require-import conversion
       build.onResolve(
         {
           filter: new RegExp(`\\.(` + allExternalTypes.join('|') + `)(\\?.*)?$`)
         },
         async ({ path: id, importer, kind }) => {
+          // if the prefix exist, it is already converted to `import`, so set `external: true`
+          if (id.startsWith(convertedExternalPrefix)) {
+            return {
+              path: id.slice(convertedExternalPrefix.length),
+              external: true
+            }
+          }
+
           const resolved = await resolve(id, importer, kind)
           if (resolved) {
+            // here it is not set to `external: true` to convert `require` to `import`
             return {
               path: resolved,
-              external: true
+              namespace: externalWithConversionNamespace
             }
           }
         }
       )
+      build.onLoad(
+        { filter: /./, namespace: externalWithConversionNamespace },
+        (args) => {
+          // import itself with prefix (this is the actual part of require-import conversion)
+          return {
+            contents:
+              `export { default } from "${convertedExternalPrefix}${args.path}";` +
+              `export * from "${convertedExternalPrefix}${args.path}";`,
+            loader: 'js'
+          }
+        }
+      )
 
       function resolveEntry(id: string) {
         const flatId = flattenId(id)

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

@@ -77,7 +77,11 @@ test('dep with dynamic import', async () => {
 })
 
 test('dep with css import', async () => {
-  expect(await getColor('h1')).toBe('red')
+  expect(await getColor('.dep-linked-include')).toBe('red')
+})
+
+test('CJS dep with css import', async () => {
+  expect(await getColor('.cjs-with-assets')).toBe('blue')
 })
 
 test('dep w/ non-js files handled via plugin', async () => {

playground/optimize-deps/dep-cjs-with-assets/foo.css

@@ -0,0 +1,3 @@
+.cjs-with-assets {
+  color: blue;
+}

playground/optimize-deps/dep-cjs-with-assets/index.js

@@ -0,0 +1,3 @@
+require('./foo.css')
+
+exports.a = 11

playground/optimize-deps/dep-cjs-with-assets/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "dep-cjs-with-assets",
+  "private": true,
+  "version": "0.0.0",
+  "main": "index.js"
+}

playground/optimize-deps/dep-linked-include/test.css

@@ -1,3 +1,3 @@
-body {
+.dep-linked-include {
   color: red;
 }

playground/optimize-deps/index.html

@@ -35,6 +35,12 @@ <h2>Detecting linked src package and optimizing its deps (lodash-es)</h2>
 <h2>Optimizing force included dep even when it's linked</h2>
 <div class="force-include"></div>
 
+<h2>Dep with CSS</h2>
+<div class="dep-linked-include">This should be red</div>
+
+<h2>CJS Dep with CSS</h2>
+<div class="cjs-with-assets">This should be blue</div>
+
 <h2>import * as ...</h2>
 <div class="import-star"></div>
 
@@ -91,6 +97,8 @@ <h2>Reused variable names</h2>
     text('.import-star', `[success] ${keys.join(', ')}`)
   }
 
+  import 'dep-cjs-with-assets'
+
   import { env } from 'dep-node-env'
   text('.node-env', env)
 

playground/optimize-deps/package.json

@@ -13,6 +13,7 @@
     "clipboard": "^2.0.11",
     "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",
     "dep-esbuild-plugin-transform": "file:./dep-esbuild-plugin-transform",
     "dep-linked": "link:./dep-linked",
     "dep-linked-include": "link:./dep-linked-include",