feat: detect and warn against imports to transitively optimized deps

close #1543
vitejs · Jan 20, 2021 · 3841e70 · 3841e70
1 parent 16393a1
commit 3841e70
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 29 deletions.
diff --git a/packages/vite/src/node/optimizer/depMetadataPlugin.ts b/packages/vite/src/node/optimizer/depMetadataPlugin.ts
@@ -1,22 +1,38 @@
 import { Plugin } from 'rollup'
 import { DepOptimizationMetadata } from '.'
 
+const moduleIdRE = /node_modules\/([^@.][^/]*|@[^/]+\/[^/]+)\//
+
 export function recordCjsEntryPlugin(data: DepOptimizationMetadata): Plugin {
   return {
     name: 'vite:cjs-entry-named-export',
-    async generateBundle(options, bundle) {
+    async generateBundle(_, bundle) {
       Object.values(bundle).forEach((chunk) => {
-        if (chunk.type === 'chunk' && chunk.isEntry) {
-          data.optimized[chunk.name] = chunk.fileName
-          if (chunk.facadeModuleId) {
-            const facadeInfo = this.getModuleInfo(chunk.facadeModuleId)
-            // this info is exposed by rollup commonjs plugin
-            if (facadeInfo?.meta?.commonjs?.isCommonJS) {
-              data.cjsEntries[chunk.fileName] = true
+        if (chunk.type === 'chunk') {
+          if (chunk.isEntry) {
+            data.optimized[chunk.name] = chunk.fileName
+            if (chunk.facadeModuleId) {
+              const facadeInfo = this.getModuleInfo(chunk.facadeModuleId)
+              // this info is exposed by rollup commonjs plugin
+              if (facadeInfo?.meta?.commonjs?.isCommonJS) {
+                data.cjsEntries[chunk.fileName] = true
+              }
+            }
+          }
+          for (const id in chunk.modules) {
+            const depId = id.match(moduleIdRE)?.[1]
+            if (depId) {
+              data.transitiveOptimized[depId] = true
             }
           }
         }
       })
+
+      for (const key in data.transitiveOptimized) {
+        if (key in data.optimized) {
+          delete data.transitiveOptimized[key]
+        }
+      }
     }
   }
 }
diff --git a/packages/vite/src/node/optimizer/index.ts b/packages/vite/src/node/optimizer/index.ts
@@ -79,6 +79,7 @@ export interface DepOptimizationOptions {
 export interface DepOptimizationMetadata {
   hash: string
   optimized: Record<string, string>
+  transitiveOptimized: Record<string, true>
   cjsEntries: Record<string, true>
   dependencies: Record<string, string>
 }
@@ -108,6 +109,7 @@ export async function optimizeDeps(
   const data: DepOptimizationMetadata = {
     hash: getDepHash(root, pkg, config),
     optimized: {},
+    transitiveOptimized: {},
     cjsEntries: {},
     dependencies: pkg.dependencies
   }

diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts
@@ -14,7 +14,8 @@ import {
   fsPathFromId,
   resolveFrom,
   isDataUrl,
-  cleanUrl
+  cleanUrl,
+  isJSRequest
 } from '../utils'
 import { ResolvedConfig, ViteDevServer } from '..'
 import slash from 'slash'
@@ -287,27 +288,44 @@ export function tryNodeResolve(
   }
 
   // prevent deep imports to optimized deps.
-  if (
-    deepMatch &&
-    server &&
-    server._optimizeDepsMetadata &&
-    pkg.data.name in server._optimizeDepsMetadata.optimized &&
-    !isCSSRequest(id) &&
-    !server.config.assetsInclude(id)
-  ) {
-    throw new Error(
-      chalk.yellow(
-        `Deep import "${chalk.cyan(
-          id
-        )}" should be avoided because dependency "${chalk.cyan(
-          pkg.data.name
-        )}" has been pre-optimized. Prefer importing directly from the module entry:\n\n` +
-          `${chalk.green(`import { ... } from "${pkg.data.name}"`)}\n\n` +
-          `If the used import is not exported from the package's main entry ` +
-          `and can only be attained via deep import, you can explicitly add ` +
-          `the deep import path to "optimizeDeps.include" in vite.config.js.`
+  if (server && server._optimizeDepsMetadata) {
+    const data = server._optimizeDepsMetadata
+    if (
+      deepMatch &&
+      pkg.data.name in data.optimized &&
+      !isCSSRequest(id) &&
+      !server.config.assetsInclude(id)
+    ) {
+      throw new Error(
+        chalk.yellow(
+          `Deep import "${chalk.cyan(
+            id
+          )}" should be avoided because dependency "${chalk.cyan(
+            pkg.data.name
+          )}" has been pre-optimized. Prefer importing directly from the module entry:\n\n` +
+            `${chalk.green(`import { ... } from "${pkg.data.name}"`)}\n\n` +
+            `If the used import is not exported from the package's main entry ` +
+            `and can only be attained via deep import, you can explicitly add ` +
+            `the deep import path to "optimizeDeps.include" in vite.config.js.`
+        )
       )
-    )
+    }
+
+    if (
+      pkgId in data.transitiveOptimized &&
+      isJSRequest(id) &&
+      basedir.startsWith(server.config.root) &&
+      !basedir.includes('node_modules')
+    ) {
+      throw new Error(
+        chalk.yellow(
+          `dependency "${chalk.bold(pkgId)}" is imported in source code, ` +
+            `but was transitively pre-bundled as part of another package. ` +
+            `It should be explicitly listed as a dependency in package.json in ` +
+            `order to avoid duplicated instances of this module.`
+        )
+      )
+    }
   }
 
   let resolved = deepMatch