From dc2deca0b78fb73dd0c859a81cba9eef3cf6772f Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sun, 8 Nov 2020 11:36:15 +0000 Subject: [PATCH 1/4] refactor handling builtins --- packages/node-resolve/src/index.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/node-resolve/src/index.js b/packages/node-resolve/src/index.js index 0c045b003..cc90eba20 100644 --- a/packages/node-resolve/src/index.js +++ b/packages/node-resolve/src/index.js @@ -197,7 +197,7 @@ export function nodeResolve(opts = {}) { const importeeIsBuiltin = builtins.has(importee); - if (importeeIsBuiltin && (!preferBuiltins || !isPreferBuiltinsSet)) { + 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 @@ -243,9 +243,7 @@ export function nodeResolve(opts = {}) { idToPackageInfo.set(resolved, packageInfo); if (hasPackageEntry) { - if (builtins.has(resolved) && preferBuiltins && isPreferBuiltinsSet) { - return null; - } else if (importeeIsBuiltin && preferBuiltins) { + 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` From b19fb3e1de786df418c6a89ecfdb70c1117ec62b Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sun, 8 Nov 2020 11:59:40 +0000 Subject: [PATCH 2/4] remove duplicate code block --- packages/node-resolve/src/index.js | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/packages/node-resolve/src/index.js b/packages/node-resolve/src/index.js index 7301145e9..659b7f564 100644 --- a/packages/node-resolve/src/index.js +++ b/packages/node-resolve/src/index.js @@ -270,19 +270,6 @@ export function nodeResolve(opts = {}) { } } - if (hasPackageEntry) { - 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)) { From 15302c6d4fcc1d12f625cd73dbf351f123ef6cec Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sun, 8 Nov 2020 12:14:29 +0000 Subject: [PATCH 3/4] do not warn when using a builtin when no local version --- packages/node-resolve/src/index.js | 2 +- .../test/fixtures/prefer-builtin-no-local.js | 1 + packages/node-resolve/test/prefer-builtins.js | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100755 packages/node-resolve/test/fixtures/prefer-builtin-no-local.js diff --git a/packages/node-resolve/src/index.js b/packages/node-resolve/src/index.js index 659b7f564..0f6a04758 100644 --- a/packages/node-resolve/src/index.js +++ b/packages/node-resolve/src/index.js @@ -259,7 +259,7 @@ export function nodeResolve(opts = {}) { if (hasPackageEntry) { if (importeeIsBuiltin && preferBuiltins) { - if (!isPreferBuiltinsSet) { + if (!isPreferBuiltinsSet && resolved !== importee) { 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` ); diff --git a/packages/node-resolve/test/fixtures/prefer-builtin-no-local.js b/packages/node-resolve/test/fixtures/prefer-builtin-no-local.js new file mode 100755 index 000000000..1ea3f7c7e --- /dev/null +++ b/packages/node-resolve/test/fixtures/prefer-builtin-no-local.js @@ -0,0 +1 @@ +import 'path'; diff --git a/packages/node-resolve/test/prefer-builtins.js b/packages/node-resolve/test/prefer-builtins.js index dda3f5008..5ff35fa95 100644 --- a/packages/node-resolve/test/prefer-builtins.js +++ b/packages/node-resolve/test/prefer-builtins.js @@ -89,3 +89,19 @@ test('false allows resolving a local module with the same name as a builtin modu t.snapshot(warnings); t.deepEqual(imports, []); }); + +test('does not warn when using a builtin module when there is no local version, no explicit configuration', async (t) => { + let warning = null; + await rollup({ + input: 'prefer-builtin-no-local.js', + onwarn({ message }) { + // eslint-disable-next-line no-bitwise + if (~message.indexOf('preferring')) { + warning = message; + } + }, + plugins: [nodeResolve()] + }); + + t.is(warning, null); +}); From 44d06251b870749cdc688997d7d6b3e3d7a494d5 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sun, 8 Nov 2020 12:19:18 +0000 Subject: [PATCH 4/4] add not about warnings to readme --- packages/node-resolve/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node-resolve/README.md b/packages/node-resolve/README.md index 0f49f6d70..2654bc496 100755 --- a/packages/node-resolve/README.md +++ b/packages/node-resolve/README.md @@ -129,7 +129,7 @@ DEPRECATED: use "resolveOnly" instead ### `preferBuiltins` Type: `Boolean`
-Default: `true` +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.