fix(importAnalysis): warning on ExportAllDeclaration (#12799)

vitejs · Apr 13, 2023 · 5136b9b · 5136b9b
1 parent 1607f4a
commit 5136b9b
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 11 deletions.
diff --git a/packages/vite/src/node/__tests__/plugins/import.spec.ts b/packages/vite/src/node/__tests__/plugins/import.spec.ts
@@ -1,9 +1,19 @@
-import { describe, expect, test } from 'vitest'
+import { beforeEach, describe, expect, test, vi } from 'vitest'
 import { transformCjsImport } from '../../plugins/importAnalysis'
 
 describe('transformCjsImport', () => {
   const url = './node_modules/.vite/deps/react.js'
   const rawUrl = 'react'
+  const config: any = {
+    command: 'serve',
+    logger: {
+      warn: vi.fn(),
+    },
+  }
+
+  beforeEach(() => {
+    config.logger.warn.mockClear()
+  })
 
   test('import specifier', () => {
     expect(
@@ -12,6 +22,8 @@ describe('transformCjsImport', () => {
         url,
         rawUrl,
         0,
+        '',
+        config,
       ),
     ).toBe(
       'import __vite__cjsImport0_react from "./node_modules/.vite/deps/react.js"; ' +
@@ -22,7 +34,14 @@ describe('transformCjsImport', () => {
 
   test('import default specifier', () => {
     expect(
-      transformCjsImport('import React from "react"', url, rawUrl, 0),
+      transformCjsImport(
+        'import React from "react"',
+        url,
+        rawUrl,
+        0,
+        '',
+        config,
+      ),
     ).toBe(
       'import __vite__cjsImport0_react from "./node_modules/.vite/deps/react.js"; ' +
         'const React = __vite__cjsImport0_react.__esModule ? __vite__cjsImport0_react.default : __vite__cjsImport0_react',
@@ -34,6 +53,8 @@ describe('transformCjsImport', () => {
         url,
         rawUrl,
         0,
+        '',
+        config,
       ),
     ).toBe(
       'import __vite__cjsImport0_react from "./node_modules/.vite/deps/react.js"; ' +
@@ -43,21 +64,48 @@ describe('transformCjsImport', () => {
 
   test('import all specifier', () => {
     expect(
-      transformCjsImport('import * as react from "react"', url, rawUrl, 0),
+      transformCjsImport(
+        'import * as react from "react"',
+        url,
+        rawUrl,
+        0,
+        '',
+        config,
+      ),
     ).toBe(
       'import __vite__cjsImport0_react from "./node_modules/.vite/deps/react.js"; ' +
         'const react = __vite__cjsImport0_react',
     )
   })
 
   test('export all specifier', () => {
-    expect(transformCjsImport('export * from "react"', url, rawUrl, 0)).toBe(
-      undefined,
+    expect(
+      transformCjsImport(
+        'export * from "react"',
+        url,
+        rawUrl,
+        0,
+        'modA',
+        config,
+      ),
+    ).toBe(undefined)
+
+    expect(config.logger.warn).toBeCalledWith(
+      expect.stringContaining(`export * from "react"\` in modA`),
     )
 
     expect(
-      transformCjsImport('export * as react from "react"', url, rawUrl, 0),
+      transformCjsImport(
+        'export * as react from "react"',
+        url,
+        rawUrl,
+        0,
+        '',
+        config,
+      ),
     ).toBe(undefined)
+
+    expect(config.logger.warn).toBeCalledTimes(1)
   })
 
   test('export name specifier', () => {
@@ -67,6 +115,8 @@ describe('transformCjsImport', () => {
         url,
         rawUrl,
         0,
+        '',
+        config,
       ),
     ).toBe(
       'import __vite__cjsImport0_react from "./node_modules/.vite/deps/react.js"; ' +
@@ -81,6 +131,8 @@ describe('transformCjsImport', () => {
         url,
         rawUrl,
         0,
+        '',
+        config,
       ),
     ).toBe(
       'import __vite__cjsImport0_react from "./node_modules/.vite/deps/react.js"; ' +
@@ -92,7 +144,14 @@ describe('transformCjsImport', () => {
 
   test('export default specifier', () => {
     expect(
-      transformCjsImport('export { default } from "react"', url, rawUrl, 0),
+      transformCjsImport(
+        'export { default } from "react"',
+        url,
+        rawUrl,
+        0,
+        '',
+        config,
+      ),
     ).toBe(
       'import __vite__cjsImport0_react from "./node_modules/.vite/deps/react.js"; ' +
         'const __vite__cjsExportDefault_0 = __vite__cjsImport0_react.__esModule ? __vite__cjsImport0_react.default : __vite__cjsImport0_react; ' +
@@ -105,6 +164,8 @@ describe('transformCjsImport', () => {
         url,
         rawUrl,
         0,
+        '',
+        config,
       ),
     ).toBe(
       'import __vite__cjsImport0_react from "./node_modules/.vite/deps/react.js"; ' +
@@ -118,6 +179,8 @@ describe('transformCjsImport', () => {
         url,
         rawUrl,
         0,
+        '',
+        config,
       ),
     ).toBe(
       'import __vite__cjsImport0_react from "./node_modules/.vite/deps/react.js"; ' +

diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts
@@ -585,7 +585,14 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
                   }
                 } else if (needsInterop) {
                   debug?.(`${url} needs interop`)
-                  interopNamedImports(str(), importSpecifier, url, index)
+                  interopNamedImports(
+                    str(),
+                    importSpecifier,
+                    url,
+                    index,
+                    importer,
+                    config,
+                  )
                   rewriteDone = true
                 }
               }
@@ -596,7 +603,14 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
                 url.includes(browserExternalId) &&
                 source.slice(expStart, start).includes('{')
               ) {
-                interopNamedImports(str(), importSpecifier, url, index)
+                interopNamedImports(
+                  str(),
+                  importSpecifier,
+                  url,
+                  index,
+                  importer,
+                  config,
+                )
                 rewriteDone = true
               }
               if (!rewriteDone) {
@@ -811,6 +825,8 @@ export function interopNamedImports(
   importSpecifier: ImportSpecifier,
   rewrittenUrl: string,
   importIndex: number,
+  importer: string,
+  config: ResolvedConfig,
 ): void {
   const source = str.original
   const {
@@ -831,7 +847,14 @@ export function interopNamedImports(
   } else {
     const exp = source.slice(expStart, expEnd)
     const rawUrl = source.slice(start, end)
-    const rewritten = transformCjsImport(exp, rewrittenUrl, rawUrl, importIndex)
+    const rewritten = transformCjsImport(
+      exp,
+      rewrittenUrl,
+      rawUrl,
+      importIndex,
+      importer,
+      config,
+    )
     if (rewritten) {
       str.overwrite(expStart, expEnd, rewritten, { contentOnly: true })
     } else {
@@ -861,6 +884,8 @@ export function transformCjsImport(
   url: string,
   rawUrl: string,
   importIndex: number,
+  importer: string,
+  config: ResolvedConfig,
 ): string | undefined {
   const node = (
     parseJS(importExp, {
@@ -869,7 +894,18 @@ export function transformCjsImport(
     }) as any
   ).body[0] as Node
 
+  // `export * from '...'` may cause unexpected problem, so give it a warning
   if (
+    config.command === 'serve' &&
+    node.type === 'ExportAllDeclaration' &&
+    !node.exported
+  ) {
+    config.logger.warn(
+      colors.yellow(
+        `\nUnable to interop \`${importExp}\` in ${importer}, this may lose module exports. Please export "${rawUrl}" as ESM or use named exports instead, e.g. \`export { A, B } from "${rawUrl}"\``,
+      ),
+    )
+  } else if (
     node.type === 'ImportDeclaration' ||
     node.type === 'ExportNamedDeclaration'
   ) {

diff --git a/packages/vite/src/node/plugins/importAnalysisBuild.ts b/packages/vite/src/node/plugins/importAnalysisBuild.ts
@@ -338,7 +338,14 @@ export function buildImportAnalysisPlugin(config: ResolvedConfig): Plugin {
                 }
               } else if (needsInterop) {
                 // config.logger.info(`${url} needs interop`)
-                interopNamedImports(str(), imports[index], url, index)
+                interopNamedImports(
+                  str(),
+                  imports[index],
+                  url,
+                  index,
+                  importer,
+                  config,
+                )
                 rewriteDone = true
               }
               if (!rewriteDone) {