refactor(optimizer): adjust node built-in handling

- prefer resolved if resolvable
- externalize to empty module when not resolved
- better plugin warning handling

docs/config/index.md

@@ -415,22 +415,10 @@ export default ({ command, mode }) => {
 
 ### optimizeDeps.exclude
 
-- **Type:** `string[]`
+- **Type:** `string | RegExp | (string | RegExp)[]`
 
   Dependencies to force exclude in pre-bundling.
 
-### optimizeDeps.link
-
-- **Type:** `string[]`
-
-  Dependencies to be explicitly treated as linked source in pre-bundling. Note Vite 2.0 automatically detects linked packages (deps whose resolved path is not inside `node_modules`) so this should only be needed in rare cases.
-
-### optimizeDeps.allowNodeBuiltins
-
-- **Type:** `string[]`
-
-  A list of dependencies that imports Node built-ins, but do not actually use them in browsers. Suppresses related warnings.
-
 ### optimizeDeps.auto
 
 - **Type:** `boolean`

packages/playground/optimize-deps-linked-include/index.js

@@ -8,3 +8,12 @@ export function useCount() {
 
 // test dep with css/asset imports
 import './test.css'
+
+// test importing node built-ins
+import fs from 'fs'
+
+if (false) {
+  fs.readFileSync()
+} else {
+  console.log('ok')
+}

packages/vite/src/node/build.ts

@@ -8,7 +8,6 @@ import Rollup, {
   RollupOptions,
   RollupWarning,
   WarningHandler,
-  WarningHandlerWithDefault,
   OutputOptions,
   RollupOutput
 } from 'rollup'
@@ -22,7 +21,6 @@ import { copyDir, emptyDir, lookupFile } from './utils'
 import { manifestPlugin } from './plugins/manifest'
 import commonjsPlugin from '@rollup/plugin-commonjs'
 import dynamicImportVars from '@rollup/plugin-dynamic-import-vars'
-import isBuiltin from 'isbuiltin'
 import { Logger } from './logger'
 import { TransformOptions } from 'esbuild'
 import { CleanCSS } from 'types/clean-css'
@@ -280,12 +278,7 @@ async function doBuild(
       ...options.rollupOptions,
       plugins: config.plugins as Plugin[],
       onwarn(warning, warn) {
-        onRollupWarning(
-          warning,
-          warn,
-          config.optimizeDeps?.allowNodeBuiltins,
-          options.rollupOptions?.onwarn
-        )
+        onRollupWarning(warning, warn, config)
       }
     })
 
@@ -397,59 +390,20 @@ const dynamicImportWarningIgnoreList = [
 export function onRollupWarning(
   warning: RollupWarning,
   warn: WarningHandler,
-  allowNodeBuiltins: string[] = [],
-  userOnWarn?: WarningHandlerWithDefault
+  config: ResolvedConfig
 ) {
-  function doWarn() {
-    if (userOnWarn) {
-      userOnWarn(warning, warn)
-    } else {
-      warn(warning)
-    }
-  }
-
   if (warning.code === 'UNRESOLVED_IMPORT') {
-    let message: string
     const id = warning.source
     const importer = warning.importer
-
-    if (importer && /\?commonjs-external$/.test(importer)) {
-      // allow commonjs external...
-      warning.message = chalk.yellow(warning.message)
-      return doWarn()
-    }
-
-    if (id && isBuiltin(id)) {
-      let importingDep: string | undefined
-      if (importer) {
-        const pkg = JSON.parse(lookupFile(importer, ['package.json']) || `{}`)
-        if (pkg.name) {
-          importingDep = pkg.name
-        }
-      }
-      if (
-        importingDep &&
-        allowNodeBuiltins.some((allowed) => importingDep!.startsWith(allowed))
-      ) {
-        return
-      }
-      const dep = importingDep
-        ? `Dependency ${chalk.yellow(importingDep)}`
-        : `A dependency`
-      message =
-        `${dep} is attempting to import Node built-in module ${chalk.yellow(
-          id
-        )}.\n` +
-        `This will not work in a browser environment.\n` +
-        `Imported by: ${chalk.gray(importer)}`
-    } else {
-      message =
-        `[vite]: Rollup failed to resolve import "${warning.source}" from "${warning.importer}".\n` +
-        `This is most likely unintended because it can break your application at runtime.\n` +
-        `If you do want to externalize this module explicitly add it to\n` +
-        `\`build.rollupOptions.external\``
+    // throw unless it's commonjs external...
+    if (!importer || !/\?commonjs-external$/.test(importer)) {
+      throw new Error(
+        `[vite]: Rollup failed to resolve import "${id}" from "${importer}".\n` +
+          `This is most likely unintended because it can break your application at runtime.\n` +
+          `If you do want to externalize this module explicitly add it to\n` +
+          `\`build.rollupOptions.external\``
+      )
     }
-    throw new Error(message)
   }
 
   if (
@@ -460,6 +414,17 @@ export function onRollupWarning(
   }
 
   if (!warningIgnoreList.includes(warning.code!)) {
-    doWarn()
+    const userOnWarn = config.build.rollupOptions?.onwarn
+    if (userOnWarn) {
+      userOnWarn(warning, warn)
+    } else if (warning.code === 'PLUGIN_WARNING') {
+      config.logger.warn(
+        `${chalk.bold.yellow(`[plugin:${warning.plugin}]`)} ${chalk.yellow(
+          warning.message
+        )}`
+      )
+    } else {
+      warn(warning)
+    }
   }
 }

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

@@ -25,6 +25,7 @@ import aliasPlugin from '@rollup/plugin-alias'
 import commonjsPlugin from '@rollup/plugin-commonjs'
 import jsonPlugin from '@rollup/plugin-json'
 import { buildDefinePlugin } from '../plugins/define'
+import { createFilter } from '@rollup/pluginutils'
 
 const debug = createDebugger('vite:optimize')
 
@@ -36,21 +37,17 @@ export interface DepOptimizationOptions {
   /**
    * Do not optimize these dependencies.
    */
-  exclude?: string[]
-  /**
-   * A list of linked dependencies that should be treated as source code.
-   */
-  link?: string[]
-  /**
-   * A list of dependencies that imports Node built-ins, but do not actually
-   * use them in browsers.
-   */
-  allowNodeBuiltins?: string[]
+  exclude?: string | RegExp | (string | RegExp)[]
   /**
    * Automatically run `vite optimize` on server start?
    * @default true
    */
   auto?: boolean
+  /**
+   * A list of linked dependencies that should be treated as source code.
+   * @deprecated
+   */
+  link?: string[]
 }
 
 export interface DepOptimizationMetadata {
@@ -174,7 +171,7 @@ export async function optimizeDeps(
       input: qualified,
       external,
       onwarn(warning, warn) {
-        onRollupWarning(warning, warn, options.allowNodeBuiltins)
+        onRollupWarning(warning, warn, config)
       },
       plugins: [
         aliasPlugin({ entries: config.alias }),
@@ -219,19 +216,6 @@ export async function optimizeDeps(
       e.message += `\n\n${chalk.cyan(
         path.relative(root, e.loc.file)
       )}\n${chalk.dim(e.frame)}`
-    } else if (e.message.match('Node built-in')) {
-      e.message += chalk.yellow(
-        `\n\nTip:\nMake sure your "dependencies" only include packages that you\n` +
-          `intend to use in the browser. If it's a Node.js package, it\n` +
-          `should be in "devDependencies".\n\n` +
-          `If you do intend to use this dependency in the browser and the\n` +
-          `dependency does not actually use these Node built-ins in the\n` +
-          `browser, you can add the dependency (not the built-in) to the\n` +
-          `"optimizeDeps.allowNodeBuiltins" option in vite.config.js.\n\n` +
-          `If that results in a runtime error, then unfortunately the\n` +
-          `package is not distributed in a web-friendly format. You should\n` +
-          `open an issue in its repo, or look for a modern alternative.`
-      )
     }
     throw e
   }
@@ -271,13 +255,14 @@ async function resolveQualifiedDeps(
   const pkg = JSON.parse(pkgContent)
   const deps = Object.keys(pkg.dependencies || {})
   const linked: string[] = []
+  const excludeFilter = exclude && createFilter(exclude)
 
   for (const id of deps) {
     if (include && include.includes(id)) {
       // already force included
       continue
     }
-    if (exclude && exclude.includes(id)) {
+    if (excludeFilter && excludeFilter(id)) {
       debug(`skipping ${id} (excluded)`)
       continue
     }

packages/vite/src/node/plugins/resolve.ts

@@ -146,14 +146,6 @@ export function resolvePlugin({
 
       // bare package imports, perform node resolve
       if (bareImportRE.test(id)) {
-        // externalize node built-ins only when building for ssr
-        if (isBuild && config && config.build.ssr && isBuiltin(id)) {
-          return {
-            id,
-            external: true
-          }
-        }
-
         if (asSrc && server && (res = tryOptimizedResolve(id, server))) {
           return res
         }
@@ -170,6 +162,23 @@ export function resolvePlugin({
         ) {
           return res
         }
+
+        // node built-ins.
+        // externalize if building for SSR, otherwise redirect to empty module
+        if (isBuiltin(id)) {
+          if (isBuild && config && config.build.ssr) {
+            return {
+              id,
+              external: true
+            }
+          } else {
+            this.warn(
+              `externalized node built-in "${id}" to empty module. ` +
+                `(imported by: ${chalk.white.dim(importer)})`
+            )
+            return browserExternalId
+          }
+        }
       }
 
       isDebug && debug(`[fallthrough] ${chalk.dim(id)}`)