From 1459cf0ab5e5eb7beee46f52bc4dbbb88d3e4335 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sun, 25 Oct 2020 13:37:41 +0000 Subject: [PATCH] fix(node-resolve): do not ignore exceptions (#564) * do not ignore errors * do not error when import specifier does not map to file * always swallow MODULE_NOT_FOUND errors as not being able to resolve the id is a valid output * return null from resolveId early if can't resolve * strip null byte from importer if there is one * fix root-dir test this was passing before because the real error was being ignored * remove try/catch * use async exists() and realpath() like we do in index.js * pretend importer is `undefined` if it starts will \0 * return null if file not found with modulesOnly option --- packages/node-resolve/src/index.js | 91 +++++++++++++------------- packages/node-resolve/src/util.js | 30 ++++----- packages/node-resolve/test/root-dir.js | 4 +- 3 files changed, 63 insertions(+), 62 deletions(-) diff --git a/packages/node-resolve/src/index.js b/packages/node-resolve/src/index.js index 65032fd84..6081a0797 100644 --- a/packages/node-resolve/src/index.js +++ b/packages/node-resolve/src/index.js @@ -100,6 +100,10 @@ export function nodeResolve(opts = {}) { // ignore IDs with null character, these belong to other plugins if (/\0/.test(importee)) return null; + if (/\0/.test(importer)) { + importer = undefined; + } + // strip query params from import const [importPath, params] = importee.split('?'); const importSuffix = `${params ? `?${params}` : ''}`; @@ -218,62 +222,61 @@ export function nodeResolve(opts = {}) { importSpecifierList.push(importee); resolveOptions = Object.assign(resolveOptions, customResolveOptions); - try { - let resolved = await resolveImportSpecifiers(importSpecifierList, resolveOptions); + let resolved = await resolveImportSpecifiers(importSpecifierList, resolveOptions); + if (!resolved) { + return null; + } - if (resolved && packageBrowserField) { - if (Object.prototype.hasOwnProperty.call(packageBrowserField, resolved)) { - if (!packageBrowserField[resolved]) { - browserMapCache.set(resolved, packageBrowserField); - return ES6_BROWSER_EMPTY; - } - resolved = packageBrowserField[resolved]; + if (packageBrowserField) { + if (Object.prototype.hasOwnProperty.call(packageBrowserField, resolved)) { + if (!packageBrowserField[resolved]) { + browserMapCache.set(resolved, packageBrowserField); + return ES6_BROWSER_EMPTY; } - browserMapCache.set(resolved, packageBrowserField); + resolved = packageBrowserField[resolved]; } + browserMapCache.set(resolved, packageBrowserField); + } - if (hasPackageEntry && !preserveSymlinks && resolved) { - const fileExists = await exists(resolved); - if (fileExists) { - resolved = await realpath(resolved); - } + if (hasPackageEntry && !preserveSymlinks) { + const fileExists = await exists(resolved); + if (fileExists) { + resolved = await realpath(resolved); } + } - idToPackageInfo.set(resolved, packageInfo); - - if (hasPackageEntry) { - if (builtins.has(resolved) && preferBuiltins && isPreferBuiltinsSet) { - return null; - } else if (importeeIsBuiltin && preferBuiltins) { - if (!isPreferBuiltinsSet) { - 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` - ); - } - return null; - } else if (jail && resolved.indexOf(normalize(jail.trim(sep))) !== 0) { - return null; - } - } + idToPackageInfo.set(resolved, packageInfo); - if (resolved && options.modulesOnly) { - const code = await readFile(resolved, 'utf-8'); - if (isModule(code)) { - return { - id: `${resolved}${importSuffix}`, - moduleSideEffects: hasModuleSideEffects(resolved) - }; + if (hasPackageEntry) { + if (builtins.has(resolved) && preferBuiltins && isPreferBuiltinsSet) { + return null; + } else if (importeeIsBuiltin && preferBuiltins) { + if (!isPreferBuiltinsSet) { + 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` + ); } return null; + } else if (jail && resolved.indexOf(normalize(jail.trim(sep))) !== 0) { + return null; + } + } + + if (options.modulesOnly && (await exists(resolved))) { + const code = await readFile(resolved, 'utf-8'); + if (isModule(code)) { + return { + id: `${resolved}${importSuffix}`, + moduleSideEffects: hasModuleSideEffects(resolved) + }; } - const result = { - id: `${resolved}${importSuffix}`, - moduleSideEffects: hasModuleSideEffects(resolved) - }; - return result; - } catch (error) { return null; } + const result = { + id: `${resolved}${importSuffix}`, + moduleSideEffects: hasModuleSideEffects(resolved) + }; + return result; }, load(importee) { diff --git a/packages/node-resolve/src/util.js b/packages/node-resolve/src/util.js index c4c8949c6..1c2628afd 100644 --- a/packages/node-resolve/src/util.js +++ b/packages/node-resolve/src/util.js @@ -5,7 +5,7 @@ import { createFilter } from '@rollup/pluginutils'; import resolveModule from 'resolve'; -import { realpathSync } from './fs'; +import { exists, realpath, realpathSync } from './fs'; const resolveId = promisify(resolveModule); @@ -165,28 +165,28 @@ export function resolveImportSpecifiers(importSpecifierList, resolveOptions) { let promise = Promise.resolve(); for (let i = 0; i < importSpecifierList.length; i++) { - promise = promise.then((value) => { + // eslint-disable-next-line no-loop-func + promise = promise.then(async (value) => { // if we've already resolved to something, just return it. if (value) { return value; } - return resolveId(importSpecifierList[i], resolveOptions).then((result) => { - if (!resolveOptions.preserveSymlinks) { - result = realpathSync(result); + let result = await resolveId(importSpecifierList[i], resolveOptions); + if (!resolveOptions.preserveSymlinks) { + if (await exists(result)) { + result = await realpath(result); } - return result; - }); + } + return result; }); - if (i < importSpecifierList.length - 1) { - // swallow MODULE_NOT_FOUND errors from all but the last resolution - promise = promise.catch((error) => { - if (error.code !== 'MODULE_NOT_FOUND') { - throw error; - } - }); - } + // swallow MODULE_NOT_FOUND errors + promise = promise.catch((error) => { + if (error.code !== 'MODULE_NOT_FOUND') { + throw error; + } + }); } return promise; diff --git a/packages/node-resolve/test/root-dir.js b/packages/node-resolve/test/root-dir.js index f200d3ca4..734823e49 100644 --- a/packages/node-resolve/test/root-dir.js +++ b/packages/node-resolve/test/root-dir.js @@ -7,11 +7,9 @@ const { testBundle } = require('../../../util/test'); const { nodeResolve } = require('..'); -process.chdir(join(__dirname, 'fixtures', 'monorepo-dedupe', 'packages', 'package-a')); - test('deduplicates modules from the given root directory', async (t) => { const bundle = await rollup({ - input: 'index.js', + input: './packages/package-a/index.js', plugins: [ nodeResolve({ dedupe: ['react'],