Skip to content
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.

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

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion packages/node-resolve/README.md
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
Expand Up @@ -55,7 +55,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 @@ -191,8 +191,10 @@ export function nodeResolve(opts = {}) {
});

const importeeIsBuiltin = isBuiltinModule(importee);
const preferImporteeIsBuiltin =
typeof preferBuiltins === 'function' ? preferBuiltins(importee) : preferBuiltins;
const resolved =
importeeIsBuiltin && preferBuiltins
importeeIsBuiltin && preferImporteeIsBuiltin
? {
packageInfo: undefined,
hasModuleSideEffects: () => null,
Expand Down Expand Up @@ -227,11 +229,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
@@ -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
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);
t.not(exports.events, require('node:events'));
t.is(exports.events, 'not the built-in events module');
});
4 changes: 3 additions & 1 deletion packages/node-resolve/types/index.d.ts
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