Skip to content

Commit

Permalink
feat(node-resolve): allow preferBuiltins to be a function (#1694)
Browse files Browse the repository at this point in the history
* feat: allow `options.preferBuiltins` to be function

* docs: update readme

* feat: add types in dts

* tests: add testcase about passing function to preferBuiltins
  • Loading branch information
younggglcy authored Sep 23, 2024
1 parent 0af45c2 commit 032055b
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 14 deletions.
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('handles importing builtins', async (t) => {
});

test('warning when preferring a builtin module, no explicit configuration', async (t) => {
let warning = null;
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 @@ test('warning when preferring a builtin module, no explicit configuration', asyn
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 @@ test('detects builtins imported with node: protocol', async (t) => {

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 / release

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 / release

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

0 comments on commit 032055b

Please sign in to comment.