fix(node-resolve): mark module as external if resolved module is external

packages/node-resolve/package.json

@@ -51,7 +51,7 @@
     "modules"
   ],
   "peerDependencies": {
-    "rollup": "^1.20.0||^2.0.0"
+    "rollup": "^2.42.0"
   },
   "dependencies": {
     "@rollup/pluginutils": "^3.1.0",
@@ -68,7 +68,7 @@
     "@rollup/plugin-commonjs": "^16.0.0",
     "@rollup/plugin-json": "^4.1.0",
     "es5-ext": "^0.10.53",
-    "rollup": "^2.23.0",
+    "rollup": "^2.42.0",
     "source-map": "^0.7.3",
     "string-capitalize": "^1.0.1"
   },

packages/node-resolve/src/index.js

@@ -72,203 +72,215 @@ export function nodeResolve(opts = {}) {
   const browserMapCache = new Map();
   let preserveSymlinks;
 
-  return {
-    name: 'node-resolve',
+  const doResolveId = async (context, importee, importer, opts) => {
+    // strip query params from import
+    const [importPath, params] = importee.split('?');
+    const importSuffix = `${params ? `?${params}` : ''}`;
+    importee = importPath;
+
+    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);
+      if (browser[importee] === false || browser[resolvedImportee] === false) {
+        return { id: ES6_BROWSER_EMPTY };
+      }
+      const browserImportee =
+        browser[importee] ||
+        browser[resolvedImportee] ||
+        browser[`${resolvedImportee}.js`] ||
+        browser[`${resolvedImportee}.json`];
+      if (browserImportee) {
+        importee = browserImportee;
+      }
+    }
 
-    buildStart(options) {
-      rollupOptions = options;
+    const parts = importee.split(/[/\\]/);
+    let id = parts.shift();
+    let isRelativeImport = false;
+
+    if (id[0] === '@' && parts.length > 0) {
+      // scoped packages
+      id += `/${parts.shift()}`;
+    } else if (id[0] === '.') {
+      // an import relative to the parent dir of the importer
+      id = resolve(baseDir, importee);
+      isRelativeImport = true;
+    }
 
-      for (const warning of warnings) {
-        this.warn(warning);
+    if (
+      !isRelativeImport &&
+      resolveOnly.length &&
+      !resolveOnly.some((pattern) => pattern.test(id))
+    ) {
+      if (normalizeInput(rollupOptions.input).includes(importee)) {
+        return null;
       }
+      return false;
+    }
 
-      ({ preserveSymlinks } = options);
-    },
+    const importSpecifierList = [];
 
-    generateBundle() {
-      readCachedFile.clear();
-      isFileCached.clear();
-      isDirCached.clear();
-    },
+    if (importer === undefined && !importee[0].match(/^\.?\.?\//)) {
+      // For module graph roots (i.e. when importer is undefined), we
+      // need to handle 'path fragments` like `foo/bar` that are commonly
+      // found in rollup config files. If importee doesn't look like a
+      // relative or absolute path, we make it relative and attempt to
+      // resolve it. If we don't find anything, we try resolving it as we
+      // got it.
+      importSpecifierList.push(`./${importee}`);
+    }
 
-    async resolveId(importee, importer, opts) {
-      if (importee === ES6_BROWSER_EMPTY) {
-        return importee;
-      }
-      // ignore IDs with null character, these belong to other plugins
-      if (/\0/.test(importee)) return null;
+    const importeeIsBuiltin = builtins.has(importee);
 
-      if (/\0/.test(importer)) {
-        importer = undefined;
-      }
+    if (importeeIsBuiltin) {
+      // 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}/`);
+    }
 
-      // strip query params from import
-      const [importPath, params] = importee.split('?');
-      const importSuffix = `${params ? `?${params}` : ''}`;
-      importee = importPath;
+    // TypeScript files may import '.js' to refer to either '.ts' or '.tsx'
+    if (importer && importee.endsWith('.js')) {
+      for (const ext of ['.ts', '.tsx']) {
+        if (importer.endsWith(ext) && extensions.includes(ext)) {
+          importSpecifierList.push(importee.replace(/.js$/, ext));
+        }
+      }
+    }
 
-      const baseDir = !importer || dedupe(importee) ? rootDir : dirname(importer);
+    importSpecifierList.push(importee);
+
+    const warn = (...args) => context.warn(...args);
+    const isRequire =
+      opts && opts.custom && opts.custom['node-resolve'] && opts.custom['node-resolve'].isRequire;
+    const exportConditions = isRequire ? conditionsCjs : conditionsEsm;
+
+    const resolvedWithoutBuiltins = await resolveImportSpecifiers({
+      importer,
+      importSpecifierList,
+      exportConditions,
+      warn,
+      packageInfoCache,
+      extensions,
+      mainFields,
+      preserveSymlinks,
+      useBrowserOverrides,
+      baseDir,
+      moduleDirectories,
+      rootDir,
+      ignoreSideEffectsForRoot
+    });
+
+    const resolved =
+      importeeIsBuiltin && preferBuiltins
+        ? {
+            packageInfo: undefined,
+            hasModuleSideEffects: () => null,
+            hasPackageEntry: true,
+            packageBrowserField: false
+          }
+        : resolvedWithoutBuiltins;
+    if (!resolved) {
+      return null;
+    }
 
-      // https://github.com/defunctzombie/package-browser-field-spec
-      const browser = browserMapCache.get(importer);
-      if (useBrowserOverrides && browser) {
-        const resolvedImportee = resolve(baseDir, importee);
-        if (browser[importee] === false || browser[resolvedImportee] === false) {
-          return ES6_BROWSER_EMPTY;
-        }
-        const browserImportee =
-          browser[importee] ||
-          browser[resolvedImportee] ||
-          browser[`${resolvedImportee}.js`] ||
-          browser[`${resolvedImportee}.json`];
-        if (browserImportee) {
-          importee = browserImportee;
+    const { packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = resolved;
+    let { location } = resolved;
+    if (packageBrowserField) {
+      if (Object.prototype.hasOwnProperty.call(packageBrowserField, location)) {
+        if (!packageBrowserField[location]) {
+          browserMapCache.set(location, packageBrowserField);
+          return { id: ES6_BROWSER_EMPTY };
         }
+        location = packageBrowserField[location];
       }
+      browserMapCache.set(location, packageBrowserField);
+    }
 
-      const parts = importee.split(/[/\\]/);
-      let id = parts.shift();
-      let isRelativeImport = false;
-
-      if (id[0] === '@' && parts.length > 0) {
-        // scoped packages
-        id += `/${parts.shift()}`;
-      } else if (id[0] === '.') {
-        // an import relative to the parent dir of the importer
-        id = resolve(baseDir, importee);
-        isRelativeImport = true;
+    if (hasPackageEntry && !preserveSymlinks) {
+      const fileExists = await exists(location);
+      if (fileExists) {
+        location = await realpath(location);
       }
+    }
 
-      if (
-        !isRelativeImport &&
-        resolveOnly.length &&
-        !resolveOnly.some((pattern) => pattern.test(id))
-      ) {
-        if (normalizeInput(rollupOptions.input).includes(importee)) {
-          return null;
+    idToPackageInfo.set(location, packageInfo);
+
+    if (hasPackageEntry) {
+      if (importeeIsBuiltin && preferBuiltins) {
+        if (!isPreferBuiltinsSet && resolvedWithoutBuiltins && resolved !== importee) {
+          context.warn(
+            `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 && location.indexOf(normalize(jail.trim(sep))) !== 0) {
+        return null;
       }
+    }
 
-      const importSpecifierList = [];
-
-      if (importer === undefined && !importee[0].match(/^\.?\.?\//)) {
-        // For module graph roots (i.e. when importer is undefined), we
-        // need to handle 'path fragments` like `foo/bar` that are commonly
-        // found in rollup config files. If importee doesn't look like a
-        // relative or absolute path, we make it relative and attempt to
-        // resolve it. If we don't find anything, we try resolving it as we
-        // got it.
-        importSpecifierList.push(`./${importee}`);
+    if (options.modulesOnly && (await exists(location))) {
+      const code = await readFile(location, 'utf-8');
+      if (isModule(code)) {
+        return {
+          id: `${location}${importSuffix}`,
+          moduleSideEffects: hasModuleSideEffects(location)
+        };
       }
+      return null;
+    }
+    const result = {
+      id: `${location}${importSuffix}`,
+      moduleSideEffects: hasModuleSideEffects(location)
+    };
+    return result;
+  };
 
-      const importeeIsBuiltin = builtins.has(importee);
+  return {
+    name: 'node-resolve',
 
-      if (importeeIsBuiltin) {
-        // 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}/`);
-      }
+    buildStart(options) {
+      rollupOptions = options;
 
-      // TypeScript files may import '.js' to refer to either '.ts' or '.tsx'
-      if (importer && importee.endsWith('.js')) {
-        for (const ext of ['.ts', '.tsx']) {
-          if (importer.endsWith(ext) && extensions.includes(ext)) {
-            importSpecifierList.push(importee.replace(/.js$/, ext));
-          }
-        }
+      for (const warning of warnings) {
+        this.warn(warning);
       }
 
-      importSpecifierList.push(importee);
-
-      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;
-
-      const resolvedWithoutBuiltins = await resolveImportSpecifiers({
-        importer,
-        importSpecifierList,
-        exportConditions,
-        warn,
-        packageInfoCache,
-        extensions,
-        mainFields,
-        preserveSymlinks,
-        useBrowserOverrides,
-        baseDir,
-        moduleDirectories,
-        rootDir,
-        ignoreSideEffectsForRoot
-      });
-
-      const resolved =
-        importeeIsBuiltin && preferBuiltins
-          ? {
-              packageInfo: undefined,
-              hasModuleSideEffects: () => null,
-              hasPackageEntry: true,
-              packageBrowserField: false
-            }
-          : resolvedWithoutBuiltins;
-      if (!resolved) {
-        return null;
-      }
+      ({ preserveSymlinks } = options);
+    },
 
-      const { packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = resolved;
-      let { location } = resolved;
-      if (packageBrowserField) {
-        if (Object.prototype.hasOwnProperty.call(packageBrowserField, location)) {
-          if (!packageBrowserField[location]) {
-            browserMapCache.set(location, packageBrowserField);
-            return ES6_BROWSER_EMPTY;
-          }
-          location = packageBrowserField[location];
-        }
-        browserMapCache.set(location, packageBrowserField);
-      }
+    generateBundle() {
+      readCachedFile.clear();
+      isFileCached.clear();
+      isDirCached.clear();
+    },
 
-      if (hasPackageEntry && !preserveSymlinks) {
-        const fileExists = await exists(location);
-        if (fileExists) {
-          location = await realpath(location);
-        }
+    async resolveId(importee, importer, opts) {
+      if (importee === ES6_BROWSER_EMPTY) {
+        return importee;
       }
+      // ignore IDs with null character, these belong to other plugins
+      if (/\0/.test(importee)) return null;
 
-      idToPackageInfo.set(location, packageInfo);
-
-      if (hasPackageEntry) {
-        if (importeeIsBuiltin && preferBuiltins) {
-          if (!isPreferBuiltinsSet && resolvedWithoutBuiltins && resolved !== importee) {
-            this.warn(
-              `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 && location.indexOf(normalize(jail.trim(sep))) !== 0) {
-          return null;
-        }
+      if (/\0/.test(importer)) {
+        importer = undefined;
       }
 
-      if (options.modulesOnly && (await exists(location))) {
-        const code = await readFile(location, 'utf-8');
-        if (isModule(code)) {
-          return {
-            id: `${location}${importSuffix}`,
-            moduleSideEffects: hasModuleSideEffects(location)
-          };
+      const resolved = await doResolveId(this, importee, importer, opts);
+      if (resolved) {
+        const resolvedResolved = await this.resolve(resolved.id, importer, { skipSelf: true });
+        const isExternal = !!(resolvedResolved && resolvedResolved.external);
+        if (isExternal) {
+          return false;
         }
-        return null;
       }
-      const result = {
-        id: `${location}${importSuffix}`,
-        moduleSideEffects: hasModuleSideEffects(location)
-      };
-      return result;
+      return resolved;
     },
 
     load(importee) {

packages/node-resolve/test/fixtures/resolved-external/main.js

@@ -0,0 +1 @@
+import 'external';