Skip to content

Node-resolve: refactor handling builtins, do not log warning if no local version #637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Nov 30, 2020
Merged

Node-resolve: refactor handling builtins, do not log warning if no local version #637

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dc2deca
refactor handling builtins
tjenkinson Nov 8, 2020
74f76ec
Merge branch 'master' of https://github.com/rollup/plugins into node-…
tjenkinson Nov 8, 2020
b19fb3e
remove duplicate code block
tjenkinson Nov 8, 2020
15302c6
do not warn when using a builtin when no local version
tjenkinson Nov 8, 2020
44d0625
add not about warnings to readme
tjenkinson Nov 8, 2020
fbf8807
Merge branch 'master' into node-resolve-refactor-builtins
tjenkinson Nov 28, 2020
e818131
Merge branch 'master' into node-resolve-refactor-builtins
shellscape Nov 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion packages/node-resolve/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,16 @@ export function nodeResolve(opts = {}) {

const importeeIsBuiltin = builtins.has(importee);

if (importeeIsBuiltin) {
// The `resolve` library will not resolve packages with the same
Copy link
Member

@lukastaegert lukastaegert Nov 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't the logic here become superfluous by using the new option in the updated resolve package?

Copy link
Member Author

@tjenkinson tjenkinson Nov 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! Am looking into that in a different branch though.

// 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']) {
Expand Down Expand Up @@ -229,7 +239,7 @@ export function nodeResolve(opts = {}) {

if (hasPackageEntry) {
if (importeeIsBuiltin && preferBuiltins) {
if (!isPreferBuiltinsSet && resolvedWithoutBuiltins) {
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`
);
Expand Down