feat: improve commonjs dependency handling

- Bump `rollup`, `@rollup/plugin-commonjs` and `@rollup/plugin-node-resolve` versions - Optimize CJS deps by default - Handle package with `jsnext` or `browser` entries - Add `rollupDedupe` option BREAKING CHANGE: The following config options have been removed: - `rollupPluginCommonJSNamedExports` - `optimizeDeps.commonJSWhitelist` CommonJS deps are now optimized by default.
vitejs · May 28, 2020 · 2f071b3 · 2f071b3
1 parent 63b0e3c
commit 2f071b3
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 119 deletions.
diff --git a/package.json b/package.json
@@ -51,9 +51,9 @@
   },
   "dependencies": {
     "@babel/parser": "^7.9.4",
-    "@rollup/plugin-commonjs": "~11.0.0",
+    "@rollup/plugin-commonjs": "^12.0.0",
     "@rollup/plugin-json": "^4.0.3",
-    "@rollup/plugin-node-resolve": "^7.1.3",
+    "@rollup/plugin-node-resolve": "^8.0.0",
     "@types/koa": "^2.11.3",
     "@types/lru-cache": "^5.1.0",
     "@vue/compiler-dom": "^3.0.0-beta.14",
@@ -86,7 +86,7 @@
     "postcss-load-config": "^2.1.0",
     "postcss-modules": "^2.0.0",
     "resolve": "^1.17.0",
-    "rollup": "^2.7.2",
+    "rollup": "^2.11.2",
     "rollup-plugin-terser": "^5.3.0",
     "rollup-plugin-vue": "^6.0.0-beta.3",
     "selfsigned": "^1.10.7",

diff --git a/src/node/build/index.ts b/src/node/build/index.ts
@@ -10,7 +10,12 @@ import {
   Plugin,
   InputOptions
 } from 'rollup'
-import { createResolver, supportedExts, InternalResolver } from '../resolver'
+import {
+  createResolver,
+  supportedExts,
+  mainFields,
+  InternalResolver
+} from '../resolver'
 import { createBuildResolvePlugin } from './buildPluginResolve'
 import { createBuildHtmlPlugin } from './buildPluginHtml'
 import { createBuildCssPlugin } from './buildPluginCss'
@@ -61,33 +66,6 @@ export const onRollupWarning: (
   }
 }
 
-/**
- * Named exports detection logic from Snowpack
- * MIT License
- * https://github.com/pikapkg/snowpack/blob/master/LICENSE
- */
-const PACKAGES_TO_AUTO_DETECT_EXPORTS = [
-  path.join('react', 'index.js'),
-  path.join('react-dom', 'index.js'),
-  'react-is',
-  'prop-types',
-  'scheduler',
-  'rxjs',
-  'exenv',
-  'body-scroll-lock'
-]
-
-function detectExports(root: string, id: string): string[] | undefined {
-  try {
-    const fileLoc = resolveFrom(root, id)
-    if (fs.existsSync(fileLoc)) {
-      return Object.keys(require(fileLoc)).filter((e) => e[0] !== '_')
-    }
-  } catch (err) {
-    // ignore
-  }
-}
-
 /**
  * Creates non-application specific plugins that are shared between the main
  * app and the dependencies. This is used by the `optimize` command to
@@ -99,14 +77,7 @@ export async function createBaseRollupPlugins(
   options: BuildConfig
 ): Promise<Plugin[]> {
   const { rollupInputOptions = {}, transforms = [] } = options
-
-  const knownNamedExports: Record<string, string[]> = {
-    ...options.rollupPluginCommonJSNamedExports
-  }
-  for (const id of PACKAGES_TO_AUTO_DETECT_EXPORTS) {
-    knownNamedExports[id] =
-      knownNamedExports[id] || detectExports(root, id) || []
-  }
+  const { nodeResolve } = require('@rollup/plugin-node-resolve')
 
   return [
     // user plugins
@@ -139,14 +110,15 @@ export async function createBaseRollupPlugins(
     }),
     // user transforms
     ...(transforms.length ? [createBuildJsTransformPlugin(transforms)] : []),
-    require('@rollup/plugin-node-resolve')({
+    nodeResolve({
       rootDir: root,
       extensions: supportedExts,
-      preferBuiltins: false
+      preferBuiltins: false,
+      dedupe: options.rollupDedupe || [],
+      mainFields
     }),
     require('@rollup/plugin-commonjs')({
-      extensions: ['.js', '.cjs'],
-      namedExports: knownNamedExports
+      extensions: ['.js', '.cjs']
     })
   ].filter(Boolean)
 }

diff --git a/src/node/config.ts b/src/node/config.ts
@@ -187,21 +187,13 @@ export interface BuildConfig extends SharedConfig {
    */
   ssr?: boolean
 
-  // The following are API only and not documented in the CLI. -----------------
+  // The following are API / config only and not documented in the CLI. --------
   /**
    * Will be passed to rollup.rollup()
    *
    * https://rollupjs.org/guide/en/#big-list-of-options
    */
   rollupInputOptions?: RollupInputOptions
-  /**
-   * Will be passed to @rollup/plugin-commonjs
-   * https://github.com/rollup/plugins/tree/commonjs-v11.1.0/packages/commonjs#namedexports
-   * This config can be removed after master branch is released.
-   * But there are some issues blocking it:
-   * https://github.com/rollup/plugins/issues/392
-   */
-  rollupPluginCommonJSNamedExports?: Record<string, string[]>
   /**
    * Will be passed to bundle.generate()
    *
@@ -214,6 +206,11 @@ export interface BuildConfig extends SharedConfig {
    * https://github.com/vuejs/rollup-plugin-vue/blob/next/src/index.ts
    */
   rollupPluginVueOptions?: Partial<RollupPluginVueOptions>
+  /**
+   * Will be passed to @rollup/plugin-node-resolve
+   * https://github.com/rollup/plugins/tree/master/packages/node-resolve#dedupe
+   */
+  rollupDedupe?: string[]
   /**
    * Whether to log asset info to console
    * @default false

diff --git a/src/node/depOptimizer.ts b/src/node/depOptimizer.ts
@@ -27,10 +27,6 @@ export interface DepOptimizationOptions {
    * Do not optimize these dependencies.
    */
   exclude?: string[]
-  /**
-   * Explicitly allow these CommonJS deps to be bundled.
-   */
-  commonJSWhitelist?: string[]
   /**
    * Automatically run `vite optimize` on server start?
    * @default true
@@ -90,15 +86,14 @@ export async function optimizeDeps(
   }
 
   const resolver = createResolver(root, config.resolvers, config.alias)
-  const { include, exclude, commonJSWhitelist } = config.optimizeDeps || {}
+  const { include, exclude } = config.optimizeDeps || {}
 
   // Determine deps to optimize. The goal is to only pre-bundle deps that falls
   // under one of the following categories:
   // 1. Has imports to relative files (e.g. lodash-es, lit-html)
   // 2. Has imports to bare modules that are not in the project's own deps
   //    (i.e. esm that imports its own dependencies, e.g. styled-components)
   await init
-  const cjsDeps: string[] = []
   const qualifiedDeps = deps.filter((id) => {
     if (include && !include.includes(id)) {
       debug(`skipping ${id} (not included)`)
@@ -108,10 +103,6 @@ export async function optimizeDeps(
       debug(`skipping ${id} (excluded)`)
       return false
     }
-    if (commonJSWhitelist && commonJSWhitelist.includes(id)) {
-      debug(`optimizing ${id} (commonJSWhitelist)`)
-      return true
-    }
     if (KNOWN_IGNORE_LIST.has(id)) {
       debug(`skipping ${id} (internal excluded)`)
       return false
@@ -121,7 +112,7 @@ export async function optimizeDeps(
       debug(`skipping ${id} (cannot resolve entry)`)
       return false
     }
-    const { entryFilePath, pkg } = pkgInfo
+    const { entryFilePath } = pkgInfo
     if (!supportedExts.includes(path.extname(entryFilePath))) {
       debug(`skipping ${id} (entry is not js)`)
       return false
@@ -138,11 +129,8 @@ export async function optimizeDeps(
     const content = fs.readFileSync(entryFilePath, 'utf-8')
     const [imports, exports] = parse(content)
     if (!exports.length && !/export\s+\*\s+from/.test(content)) {
-      if (!pkg.module) {
-        cjsDeps.push(id)
-      }
-      debug(`skipping ${id} (no exports, likely commonjs)`)
-      return false
+      debug(`optimizing ${id} (no exports, likely commonjs)`)
+      return true
     }
     for (const { s, e } of imports) {
       let i = content.slice(s, e).trim()
@@ -160,37 +148,16 @@ export async function optimizeDeps(
   })
 
   if (!qualifiedDeps.length) {
-    if (!cjsDeps.length) {
-      await fs.writeFile(hashPath, depHash)
-      log(`No listed dependency requires optimization. Skipping.`)
-    } else {
-      console.error(
-        chalk.yellow(
-          `[vite] The following dependencies seem to be CommonJS modules that\n` +
-            `do not provide ESM-friendly file formats:\n\n  ` +
-            cjsDeps.map((dep) => chalk.magenta(dep)).join(`\n  `) +
-            `\n` +
-            `\n- If you are not using them in browser code, you can move them\n` +
-            `to devDependencies or exclude them from this check by adding\n` +
-            `them to ${chalk.cyan(
-              `optimizeDeps.exclude`
-            )} in vue.config.js.\n` +
-            `\n- If you do intend to use them in the browser, you can try adding\n` +
-            `them to ${chalk.cyan(
-              `optimizeDeps.commonJSWhitelist`
-            )} in vue.config.js but they\n` +
-            `may fail to bundle or work properly. Consider choosing more modern\n` +
-            `alternatives that provide ES module build formts.`
-        )
-      )
-    }
+    await fs.writeFile(hashPath, depHash)
+    log(`No listed dependency requires optimization. Skipping.`)
     return
   }
 
   if (!asCommand) {
     // This is auto run on server start - let the user know that we are
     // pre-optimizing deps
-    console.log(chalk.greenBright(`[vite] Optimizable dependencies detected.`))
+    console.log(chalk.greenBright(`[vite] Optimizable dependencies detected:`))
+    console.log(qualifiedDeps.map((id) => chalk.yellow(id)).join(`, `))
   }
 
   let spinner: Ora | undefined
@@ -241,25 +208,15 @@ export async function optimizeDeps(
 
     spinner && spinner.stop()
 
-    const optimized = []
     for (const chunk of output) {
       if (chunk.type === 'chunk') {
         const fileName = chunk.fileName
         const filePath = path.join(cacheDir, fileName)
         await fs.ensureDir(path.dirname(filePath))
         await fs.writeFile(filePath, chunk.code)
-        if (!fileName.startsWith('common/')) {
-          optimized.push(fileName.replace(/\.js$/, ''))
-        }
       }
     }
 
-    console.log(
-      `Optimized modules:\n${optimized
-        .map((id) => chalk.yellowBright(id))
-        .join(`, `)}`
-    )
-
     await fs.writeFile(hashPath, depHash)
   } catch (e) {
     spinner && spinner.stop()

diff --git a/src/node/resolver.ts b/src/node/resolver.ts
@@ -26,6 +26,7 @@ export interface InternalResolver {
 }
 
 export const supportedExts = ['.mjs', '.js', '.ts', '.jsx', '.tsx', '.json']
+export const mainFields = ['module', 'jsnext', 'jsnext:main', 'browser', 'main']
 
 const defaultRequestToFile = (publicPath: string, root: string): string => {
   if (moduleRE.test(publicPath)) {
@@ -261,7 +262,7 @@ export function resolveNodeModule(
     } catch (e) {
       return
     }
-    let entryPoint: string | undefined
+    let entryPoint: string | null = null
     if (pkg.exports) {
       if (typeof pkg.exports === 'string') {
         entryPoint = pkg.exports
@@ -274,7 +275,12 @@ export function resolveNodeModule(
       }
     }
     if (!entryPoint) {
-      entryPoint = pkg.module || pkg.main || null
+      for (const field of mainFields) {
+        if (pkg[field]) {
+          entryPoint = pkg[field]
+          break
+        }
+      }
     }
 
     debug(`(node_module entry) ${id} -> ${entryPoint}`)

diff --git a/yarn.lock b/yarn.lock
@@ -465,13 +465,15 @@
   resolved "https://registry.yarnpkg.com/@pika/react/-/react-16.13.1.tgz#20e47997d2a2f1e5da39a8e28b75db2ec77d99c6"
   integrity sha512-v33Ub2QxntNpDFRnkj3tCbT6jMb7Etu7LOMQO/YAulLRIDtDvJdMwuOVJDdPYUmDtWjfWOB5xSP7nl7k0BApbQ==
 
-"@rollup/plugin-commonjs@~11.0.0":
-  version "11.0.2"
-  resolved "https://registry.yarnpkg.com/@rollup/plugin-commonjs/-/plugin-commonjs-11.0.2.tgz#837cc6950752327cb90177b608f0928a4e60b582"
-  integrity sha512-MPYGZr0qdbV5zZj8/2AuomVpnRVXRU5XKXb3HVniwRoRCreGlf5kOE081isNWeiLIi6IYkwTX9zE0/c7V8g81g==
+"@rollup/plugin-commonjs@^12.0.0":
+  version "12.0.0"
+  resolved "https://registry.yarnpkg.com/@rollup/plugin-commonjs/-/plugin-commonjs-12.0.0.tgz#e2f308ae6057499e0f413f878fff7c3a0fdc02a1"
+  integrity sha512-8+mDQt1QUmN+4Y9D3yCG8AJNewuTSLYPJVzKKUZ+lGeQrI+bV12Tc5HCyt2WdlnG6ihIL/DPbKRJlB40DX40mw==
   dependencies:
-    "@rollup/pluginutils" "^3.0.0"
+    "@rollup/pluginutils" "^3.0.8"
+    commondir "^1.0.1"
     estree-walker "^1.0.1"
+    glob "^7.1.2"
     is-reference "^1.1.2"
     magic-string "^0.25.2"
     resolve "^1.11.0"
@@ -483,18 +485,20 @@
   dependencies:
     "@rollup/pluginutils" "^3.0.8"
 
-"@rollup/plugin-node-resolve@^7.1.3":
-  version "7.1.3"
-  resolved "https://registry.yarnpkg.com/@rollup/plugin-node-resolve/-/plugin-node-resolve-7.1.3.tgz#80de384edfbd7bfc9101164910f86078151a3eca"
-  integrity sha512-RxtSL3XmdTAE2byxekYLnx+98kEUOrPHF/KRVjLH+DEIHy6kjIw7YINQzn+NXiH/NTrQLAwYs0GWB+csWygA9Q==
+"@rollup/plugin-node-resolve@^8.0.0":
+  version "8.0.0"
+  resolved "https://registry.yarnpkg.com/@rollup/plugin-node-resolve/-/plugin-node-resolve-8.0.0.tgz#47cc0775e31b6a531c88a40270377fe899a271cb"
+  integrity sha512-5poJCChrkVggXXND/sQ7yNqwjUNT4fP31gpRWCnSNnlXuUXTCMHT33xZrTGxgjm5Rl18MHj7iEzlCT8rYWwQSA==
   dependencies:
     "@rollup/pluginutils" "^3.0.8"
     "@types/resolve" "0.0.8"
     builtin-modules "^3.1.0"
+    deep-freeze "^0.0.1"
+    deepmerge "^4.2.2"
     is-module "^1.0.0"
     resolve "^1.14.2"
 
-"@rollup/pluginutils@^3.0.0", "@rollup/pluginutils@^3.0.8":
+"@rollup/pluginutils@^3.0.8":
   version "3.0.10"
   resolved "https://registry.yarnpkg.com/@rollup/pluginutils/-/pluginutils-3.0.10.tgz#a659b9025920378494cd8f8c59fbf9b3a50d5f12"
   integrity sha512-d44M7t+PjmMrASHbhgpSbVgtL6EFyX7J4mYxwQ/c5eoaE6N2VgCgEcWVzNnwycIloti+/MpwFr8qfw+nRw00sw==
@@ -1745,6 +1749,11 @@ commander@^5.0.0:
   resolved "https://registry.yarnpkg.com/commander/-/commander-5.1.0.tgz#46abbd1652f8e059bddaef99bbdcb2ad9cf179ae"
   integrity sha512-P0CysNDQ7rtVw4QIQtm+MRxV66vKFSvlsQvGYXZWR3qFU0jlMKHZZZgw8e+8DSah4UDKMqnknRDQz+xuQXQ/Zg==
 
+commondir@^1.0.1:
+  version "1.0.1"
+  resolved "https://registry.yarnpkg.com/commondir/-/commondir-1.0.1.tgz#ddd800da0c66127393cca5950ea968a3aaf1253b"
+  integrity sha1-3dgA2gxmEnOTzKWVDqloo6rxJTs=
+
 compare-func@^1.3.1:
   version "1.3.2"
   resolved "https://registry.yarnpkg.com/compare-func/-/compare-func-1.3.2.tgz#99dd0ba457e1f9bc722b12c08ec33eeab31fa648"
@@ -2299,6 +2308,11 @@ deep-equal@~1.0.1:
   resolved "https://registry.yarnpkg.com/deep-equal/-/deep-equal-1.0.1.tgz#f5d260292b660e084eff4cdbc9f08ad3247448b5"
   integrity sha1-9dJgKStmDghO/0zbyfCK0yR0SLU=
 
+deep-freeze@^0.0.1:
+  version "0.0.1"
+  resolved "https://registry.yarnpkg.com/deep-freeze/-/deep-freeze-0.0.1.tgz#3a0b0005de18672819dfd38cd31f91179c893e84"
+  integrity sha1-OgsABd4YZygZ39OM0x+RF5yJPoQ=
+
 deep-is@~0.1.3:
   version "0.1.3"
   resolved "https://registry.yarnpkg.com/deep-is/-/deep-is-0.1.3.tgz#b369d6fb5dbc13eecf524f91b070feedc357cf34"
@@ -6316,10 +6330,10 @@ rollup-pluginutils@^2.8.2:
   dependencies:
     estree-walker "^0.6.1"
 
-rollup@^2.7.2:
-  version "2.7.6"
-  resolved "https://registry.yarnpkg.com/rollup/-/rollup-2.7.6.tgz#8e6682e64ca65eb33b896dcce902696f0415ce1a"
-  integrity sha512-AdHosxHBKyBsdtbT1/AqbWNQ87O4SSxS4N9iMwEpoCDAT6e4Du3uJSy83mp3ckgmCxly5VeXGx0WHsm21Djytg==
+rollup@^2.11.2:
+  version "2.11.2"
+  resolved "https://registry.yarnpkg.com/rollup/-/rollup-2.11.2.tgz#b1273ad3e43953d09807672f6b72d08f9f9bc008"
+  integrity sha512-pJT6mfH+/gh1sOWyNMAWxjbYGL5x2AfsaR0SWLRwq2e7vxOKt/0mBjtYDTVYF8JXxVzmnuDzA+EpsPLWt/oyrg==
   optionalDependencies:
     fsevents "~2.1.2"