refactor(node-resolve)!: simplify builtins and remove `customResolveO…

…ptions` (#656) BREAKING CHANGES: See #656 * refactor handling builtins * remove duplicate code block * do not warn when using a builtin when no local version * add not about warnings to readme * update node-resolve, use `includeCoreModules`, and simplify this also removes `customResolveOptions` and adds `moduleDirectory` * remove console log * remove deprecated `only` option * remove `only` from types * update types test * lint index.d.ts * disable spaced-comment rule because it's breaking on .d.ts files * mention rollup `preserveSymlinks` option in readme * moduleDirectory => moduleDirectories * put filter in config object directly * add missing options to types.ts * add deprecation warnings/errors * handle deprecations in separate file * move catch closer to error * revert commonjs changes looks like auto lint messed up * remove old comment Co-authored-by: Andrew Powell <shellscape@users.noreply.github.com>
rollup · Nov 30, 2020 · 23b0bf7 · 23b0bf7
1 parent 094b105
commit 23b0bf7
Show file tree

Hide file tree

Showing 27 changed files with 421 additions and 218 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -33,6 +33,7 @@ module.exports = {
     'import/no-namespace': 'off',
     'import/no-named-export': 'off',
     'no-unused-vars': 'off',
+    'spaced-comment': 'off',
     'prettier/prettier': [
       'error',
       {

diff --git a/packages/dynamic-import-vars/README.md b/packages/dynamic-import-vars/README.md
@@ -40,8 +40,8 @@ export default {
   plugins: [
     dynamicImportVars({
       // options
-    })
-  ]
+    }),
+  ],
 };
 ```
 

diff --git a/packages/node-resolve/README.md b/packages/node-resolve/README.md
@@ -62,18 +62,12 @@ Default: `false`
 
 If `true`, instructs the plugin to use the `"browser"` property in `package.json` files to specify alternative files to load for bundling. This is useful when bundling for a browser environment. Alternatively, a value of `'browser'` can be added to the `mainFields` option. If `false`, any `"browser"` properties in package files will be ignored. This option takes precedence over `mainFields`.
 
-### `customResolveOptions`
+### `moduleDirectories`
 
-Type: `Object`<br>
-Default: `null`
-
-An `Object` that specifies additional options that should be passed through to [`resolve`](https://www.npmjs.com/package/resolve).
+Type: `Array[...String]`<br>
+Default: `['node_modules']`
 
-```
-customResolveOptions: {
-  moduleDirectory: 'js_modules'
-}
-```
+One or more directories in which to recursively look for modules.
 
 ### `dedupe`
 
@@ -122,13 +116,10 @@ Valid values: `['browser', 'jsnext:main', 'module', 'main']`
 
 Specifies the properties to scan within a `package.json`, used to determine the bundle entry point. The order of property names is significant, as the first-found property is used as the resolved entry point. If the array contains `'browser'`, key/values specified in the `package.json` `browser` property will be used.
 
-### `only`
-
-DEPRECATED: use "resolveOnly" instead
-
 ### `preferBuiltins`
 
 Type: `Boolean`<br>
+Default: `true` (with warnings if a builtin module is used over a local version. Set to `true` to disable warning.)
 
 If `true`, the plugin will prefer built-in modules (e.g. `fs`, `path`). If `false`, the plugin will look for locally installed modules of the same name.
 
@@ -160,6 +151,10 @@ Specifies the root directory from which to resolve modules. Typically used when
 rootDir: path.join(process.cwd(), '..')
 ```
 
+## Preserving symlinks
+
+This plugin honours the rollup [`preserveSymlinks`](https://rollupjs.org/guide/en/#preservesymlinks) option.
+
 ## Using with @rollup/plugin-commonjs
 
 Since most packages in your node_modules folder are probably legacy CommonJS rather than JavaScript modules, you may need to use [@rollup/plugin-commonjs](https://github.com/rollup/plugins/tree/master/packages/commonjs):

diff --git a/packages/node-resolve/package.json b/packages/node-resolve/package.json
@@ -59,7 +59,7 @@
     "builtin-modules": "^3.1.0",
     "deepmerge": "^4.2.2",
     "is-module": "^1.0.0",
-    "resolve": "^1.17.0"
+    "resolve": "^1.19.0"
   },
   "devDependencies": {
     "@babel/core": "^7.10.5",

diff --git a/packages/node-resolve/src/deprecated-options.js b/packages/node-resolve/src/deprecated-options.js
@@ -0,0 +1,46 @@
+export default function handleDeprecatedOptions(opts) {
+  const warnings = [];
+
+  if (opts.customResolveOptions) {
+    const { customResolveOptions } = opts;
+    if (customResolveOptions.moduleDirectory) {
+      // eslint-disable-next-line no-param-reassign
+      opts.moduleDirectories = Array.isArray(customResolveOptions.moduleDirectory)
+        ? customResolveOptions.moduleDirectory
+        : [customResolveOptions.moduleDirectory];
+
+      warnings.push(
+        'node-resolve: The `customResolveOptions.moduleDirectory` option has been deprecated. Use `moduleDirectories`, which must be an array.'
+      );
+    }
+
+    if (customResolveOptions.preserveSymlinks) {
+      throw new Error(
+        'node-resolve: `customResolveOptions.preserveSymlinks` is no longer an option. We now always use the rollup `preserveSymlinks` option.'
+      );
+    }
+
+    [
+      'basedir',
+      'package',
+      'extensions',
+      'includeCoreModules',
+      'readFile',
+      'isFile',
+      'isDirectory',
+      'realpath',
+      'packageFilter',
+      'pathFilter',
+      'paths',
+      'packageIterator'
+    ].forEach((resolveOption) => {
+      if (customResolveOptions[resolveOption]) {
+        throw new Error(
+          `node-resolve: \`customResolveOptions.${resolveOption}\` is no longer an option. If you need this, please open an issue.`
+        );
+      }
+    });
+  }
+
+  return { warnings };
+}
diff --git a/packages/node-resolve/src/index.js b/packages/node-resolve/src/index.js
@@ -8,11 +8,11 @@ import isModule from 'is-module';
 import { isDirCached, isFileCached, readCachedFile } from './cache';
 import { exists, readFile, realpath } from './fs';
 import { resolveImportSpecifiers } from './resolveImportSpecifiers';
-import { getMainFields, getPackageInfo, getPackageName, normalizeInput } from './util';
+import { getMainFields, getPackageName, normalizeInput } from './util';
+import handleDeprecatedOptions from './deprecated-options';
 
 const builtins = new Set(builtinList);
 const ES6_BROWSER_EMPTY = '\0node-resolve:empty.js';
-const nullFn = () => null;
 const deepFreeze = (object) => {
   Object.freeze(object);
 
@@ -29,21 +29,22 @@ const baseConditions = ['default', 'module'];
 const baseConditionsEsm = [...baseConditions, 'import'];
 const baseConditionsCjs = [...baseConditions, 'require'];
 const defaults = {
-  customResolveOptions: {},
   dedupe: [],
   // It's important that .mjs is listed before .js so that Rollup will interpret npm modules
   // which deploy both ESM .mjs and CommonJS .js files as ESM.
   extensions: ['.mjs', '.js', '.json', '.node'],
-  resolveOnly: []
+  resolveOnly: [],
+  moduleDirectories: ['node_modules']
 };
 export const DEFAULTS = deepFreeze(deepMerge({}, defaults));
 
 export function nodeResolve(opts = {}) {
-  const options = Object.assign({}, defaults, opts);
-  const { customResolveOptions, extensions, jail } = options;
+  const { warnings } = handleDeprecatedOptions(opts);
+
+  const options = { ...defaults, ...opts };
+  const { extensions, jail, moduleDirectories } = options;
   const conditionsEsm = [...baseConditionsEsm, ...(options.exportConditions || [])];
   const conditionsCjs = [...baseConditionsCjs, ...(options.exportConditions || [])];
-  const warnings = [];
   const packageInfoCache = new Map();
   const idToPackageInfo = new Map();
   const mainFields = getMainFields(options);
@@ -54,11 +55,6 @@ export function nodeResolve(opts = {}) {
   let { dedupe } = options;
   let rollupOptions;
 
-  if (options.only) {
-    warnings.push('node-resolve: The `only` options is deprecated, please use `resolveOnly`');
-    options.resolveOnly = options.only;
-  }
-
   if (typeof dedupe !== 'function') {
     dedupe = (importee) =>
       options.dedupe.includes(importee) || options.dedupe.includes(getPackageName(importee));
@@ -110,12 +106,12 @@ export function nodeResolve(opts = {}) {
       const importSuffix = `${params ? `?${params}` : ''}`;
       importee = importPath;
 
-      const basedir = !importer || dedupe(importee) ? rootDir : dirname(importer);
+      const baseDir = !importer || dedupe(importee) ? rootDir : dirname(importer);
 
       // https://github.com/defunctzombie/package-browser-field-spec
       const browser = browserMapCache.get(importer);
       if (useBrowserOverrides && browser) {
-        const resolvedImportee = resolve(basedir, importee);
+        const resolvedImportee = resolve(baseDir, importee);
         if (browser[importee] === false || browser[resolvedImportee] === false) {
           return ES6_BROWSER_EMPTY;
         }
@@ -138,7 +134,7 @@ export function nodeResolve(opts = {}) {
         id += `/${parts.shift()}`;
       } else if (id[0] === '.') {
         // an import relative to the parent dir of the importer
-        id = resolve(basedir, importee);
+        id = resolve(baseDir, importee);
         isRelativeImport = true;
       }
 
@@ -153,40 +149,6 @@ export function nodeResolve(opts = {}) {
         return false;
       }
 
-      let hasModuleSideEffects = nullFn;
-      let hasPackageEntry = true;
-      let packageBrowserField = false;
-      let packageInfo;
-
-      const filter = (pkg, pkgPath) => {
-        const info = getPackageInfo({
-          cache: packageInfoCache,
-          extensions,
-          pkg,
-          pkgPath,
-          mainFields,
-          preserveSymlinks,
-          useBrowserOverrides
-        });
-
-        ({ packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = info);
-
-        return info.cachedPkg;
-      };
-
-      let resolveOptions = {
-        basedir,
-        packageFilter: filter,
-        readFile: readCachedFile,
-        isFile: isFileCached,
-        isDirectory: isDirCached,
-        extensions
-      };
-
-      if (preserveSymlinks !== undefined) {
-        resolveOptions.preserveSymlinks = preserveSymlinks;
-      }
-
       const importSpecifierList = [];
 
       if (importer === undefined && !importee[0].match(/^\.?\.?\//)) {
@@ -201,16 +163,6 @@ export function nodeResolve(opts = {}) {
 
       const importeeIsBuiltin = builtins.has(importee);
 
-      if (importeeIsBuiltin && (!preferBuiltins || !isPreferBuiltinsSet)) {
-        // The `resolve` library will not resolve packages with the same
-        // name as a node built-in module. If we're resolving something
-        // that's a builtin, and we don't prefer to find built-ins, we
-        // first try to look up a local module with that name. If we don't
-        // find anything, we resolve the builtin which just returns back
-        // the built-in's name.
-        importSpecifierList.push(`${importee}/`);
-      }
-
       // TypeScript files may import '.js' to refer to either '.ts' or '.tsx'
       if (importer && importee.endsWith('.js')) {
         for (const ext of ['.ts', '.tsx']) {
@@ -221,70 +173,86 @@ export function nodeResolve(opts = {}) {
       }
 
       importSpecifierList.push(importee);
-      resolveOptions = Object.assign(resolveOptions, customResolveOptions);
 
       const warn = (...args) => this.warn(...args);
       const isRequire =
         opts && opts.custom && opts.custom['node-resolve'] && opts.custom['node-resolve'].isRequire;
       const exportConditions = isRequire ? conditionsCjs : conditionsEsm;
-      let resolved = await resolveImportSpecifiers(
+
+      const resolvedWithoutBuiltins = await resolveImportSpecifiers({
         importSpecifierList,
-        resolveOptions,
         exportConditions,
-        warn
-      );
+        warn,
+        packageInfoCache,
+        extensions,
+        mainFields,
+        preserveSymlinks,
+        useBrowserOverrides,
+        baseDir,
+        moduleDirectories
+      });
+
+      const resolved =
+        importeeIsBuiltin && preferBuiltins
+          ? {
+              packageInfo: undefined,
+              hasModuleSideEffects: () => null,
+              hasPackageEntry: true,
+              packageBrowserField: false
+            }
+          : resolvedWithoutBuiltins;
       if (!resolved) {
         return null;
       }
 
+      const { packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = resolved;
+      let { location } = resolved;
       if (packageBrowserField) {
-        if (Object.prototype.hasOwnProperty.call(packageBrowserField, resolved)) {
-          if (!packageBrowserField[resolved]) {
-            browserMapCache.set(resolved, packageBrowserField);
+        if (Object.prototype.hasOwnProperty.call(packageBrowserField, location)) {
+          if (!packageBrowserField[location]) {
+            browserMapCache.set(location, packageBrowserField);
             return ES6_BROWSER_EMPTY;
           }
-          resolved = packageBrowserField[resolved];
+          location = packageBrowserField[location];
         }
-        browserMapCache.set(resolved, packageBrowserField);
+        browserMapCache.set(location, packageBrowserField);
       }
 
       if (hasPackageEntry && !preserveSymlinks) {
-        const fileExists = await exists(resolved);
+        const fileExists = await exists(location);
         if (fileExists) {
-          resolved = await realpath(resolved);
+          location = await realpath(location);
         }
       }
 
-      idToPackageInfo.set(resolved, packageInfo);
+      idToPackageInfo.set(location, packageInfo);
 
       if (hasPackageEntry) {
-        if (builtins.has(resolved) && preferBuiltins && isPreferBuiltinsSet) {
-          return false;
-        } else if (importeeIsBuiltin && preferBuiltins) {
-          if (!isPreferBuiltinsSet) {
+        if (importeeIsBuiltin && preferBuiltins) {
+          if (!isPreferBuiltinsSet && resolvedWithoutBuiltins) {
             this.warn(
-              `preferring built-in module '${importee}' over local alternative at '${resolved}', pass 'preferBuiltins: false' to disable this behavior or 'preferBuiltins: true' to disable this warning`
+              `preferring built-in module '${importee}' over local alternative at '${resolvedWithoutBuiltins.location}', pass 'preferBuiltins: false' to disable this behavior or 'preferBuiltins: true' to disable this warning`
             );
           }
           return false;
-        } else if (jail && resolved.indexOf(normalize(jail.trim(sep))) !== 0) {
+        } else if (jail && location.indexOf(normalize(jail.trim(sep))) !== 0) {
           return null;
         }
       }
 
-      if (options.modulesOnly && (await exists(resolved))) {
-        const code = await readFile(resolved, 'utf-8');
+      if (options.modulesOnly && (await exists(location))) {
+        const code = await readFile(location, 'utf-8');
         if (isModule(code)) {
           return {
-            id: `${resolved}${importSuffix}`,
-            moduleSideEffects: hasModuleSideEffects(resolved)
+            id: `${location}${importSuffix}`,
+            moduleSideEffects: hasModuleSideEffects(location)
           };
         }
         return null;
       }
       const result = {
-        id: `${resolved}${importSuffix}`,
-        moduleSideEffects: hasModuleSideEffects(resolved)
+        id: `${location}${importSuffix}`,
+        moduleSideEffects: hasModuleSideEffects(location)
       };
       return result;
     },
-Original file line number
+Diff line change
@@ Expand Up / @@ -40,8 +40,8 @@ export default { @@
       plugins: [
         dynamicImportVars({
           // options
-        })
-      ]
+        }),
+      ],
     };
     ```
@@ Expand Down @@