Skip to content
New issue

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

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

Already on GitHub? # to your account

feat(node-resolve): allow preferBuiltins to be a function #1694

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion packages/node-resolve/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,19 @@ Specifies the properties to scan within a `package.json`, used to determine the

### `preferBuiltins`

Type: `Boolean`<br>
Type: `Boolean | (module: string) => 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.

Alternatively, you may pass in a function that returns a boolean to confirm whether the plugin should prefer built-in modules. e.g.

```js
preferBuiltins: (module) => module !== 'punycode';
```

will not treat `punycode` as a built-in module

### `modulesOnly`

Type: `Boolean`<br>
Expand Down
17 changes: 11 additions & 6 deletions packages/node-resolve/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export function nodeResolve(opts = {}) {
const idToPackageInfo = new Map();
const mainFields = getMainFields(options);
const useBrowserOverrides = mainFields.indexOf('browser') !== -1;
const isPreferBuiltinsSet = options.preferBuiltins === true || options.preferBuiltins === false;
const isPreferBuiltinsSet = Object.prototype.hasOwnProperty.call(options, 'preferBuiltins');
const preferBuiltins = isPreferBuiltinsSet ? options.preferBuiltins : true;
const rootDir = resolve(options.rootDir || process.cwd());
let { dedupe } = options;
Expand Down Expand Up @@ -194,8 +194,10 @@ export function nodeResolve(opts = {}) {
});

const importeeIsBuiltin = builtinModules.includes(importee.replace(nodeImportPrefix, ''));
const preferImporteeIsBuiltin =
typeof preferBuiltins === 'function' ? preferBuiltins(importee) : preferBuiltins;
const resolved =
importeeIsBuiltin && preferBuiltins
importeeIsBuiltin && preferImporteeIsBuiltin
? {
packageInfo: undefined,
hasModuleSideEffects: () => null,
Expand Down Expand Up @@ -230,11 +232,14 @@ export function nodeResolve(opts = {}) {
idToPackageInfo.set(location, packageInfo);

if (hasPackageEntry) {
if (importeeIsBuiltin && preferBuiltins) {
if (importeeIsBuiltin && preferImporteeIsBuiltin) {
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`
);
context.warn({
message:
`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.` +
`or passing a function to 'preferBuiltins' to provide more fine-grained control over which built-in modules to prefer.`,
pluginCode: 'PREFER_BUILTINS'
});
}
return false;
} else if (jail && location.indexOf(normalize(jail.trim(sep))) !== 0) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { sep } from 'path';
import events from 'events';

export default { sep, events };
36 changes: 30 additions & 6 deletions packages/node-resolve/test/prefer-builtins.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,12 @@
});

test('warning when preferring a builtin module, no explicit configuration', async (t) => {
let warning = null;
younggglcy marked this conversation as resolved.
Show resolved Hide resolved
let warning = '';
await rollup({
input: 'prefer-builtin.js',
onwarn({ message }) {
// eslint-disable-next-line no-bitwise
if (~message.indexOf('preferring')) {
warning = message;
onwarn({ message, pluginCode }) {
if (pluginCode === 'PREFER_BUILTINS') {
warning += message;
}
},
plugins: [nodeResolve()]
Expand All @@ -47,7 +46,8 @@
warning,
`preferring built-in module 'events' over local alternative ` +
`at '${localPath}', pass 'preferBuiltins: false' to disable this behavior ` +
`or 'preferBuiltins: true' to disable this warning`
`or 'preferBuiltins: true' to disable this warning.` +
`or passing a function to 'preferBuiltins' to provide more fine-grained control over which built-in modules to prefer.`
);
});

Expand Down Expand Up @@ -134,3 +134,27 @@

t.is(warnings.length, 0);
});

test('accpet passing a function to determine which builtins to prefer', async (t) => {
const warnings = [];
const bundle = await rollup({
input: 'prefer-builtin-local-and-builtin.js',
onwarn({ message }) {
warnings.push(message);
},
plugins: [
nodeResolve({
preferBuiltins: (id) => id !== 'events'
})
]
});

const {
module: { exports }
} = await testBundle(t, bundle);

t.is(warnings.length, 0);
t.is(exports.sep, require('node:path').sep);

Check warning on line 157 in packages/node-resolve/test/prefer-builtins.js

View workflow job for this annotation

GitHub Actions / Node v20

Unexpected require()

Check warning on line 157 in packages/node-resolve/test/prefer-builtins.js

View workflow job for this annotation

GitHub Actions / Node v18

Unexpected require()
t.not(exports.events, require('node:events'));

Check warning on line 158 in packages/node-resolve/test/prefer-builtins.js

View workflow job for this annotation

GitHub Actions / Node v20

Unexpected require()

Check warning on line 158 in packages/node-resolve/test/prefer-builtins.js

View workflow job for this annotation

GitHub Actions / Node v18

Unexpected require()
t.is(exports.events, 'not the built-in events module');
});
4 changes: 3 additions & 1 deletion packages/node-resolve/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,11 @@ export interface RollupNodeResolveOptions {
/**
* 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.
*
* If a function is provided, it will be called to determine whether to prefer built-ins.
* @default true
*/
preferBuiltins?: boolean;
preferBuiltins?: boolean | ((module: string) => boolean);

/**
* An `Array` which instructs the plugin to limit module resolution to those whose
Expand Down
Loading