Skip to content

Commit

Permalink
fix: use empty polyfill if browser field is false (#151)
Browse files Browse the repository at this point in the history
  • Loading branch information
markdalgleish authored Aug 24, 2023
1 parent 8681bbb commit 3946db8
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
15 changes: 14 additions & 1 deletion src/lib/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,20 @@ export const nodeModulesPolyfillPlugin = (options: NodePolyfillsOptions = {}): P
if (initialOptions.platform === 'browser') {
const packageJson = await loadPackageJSON(args.resolveDir);
const browserFieldValue = packageJson?.browser?.[args.path];
if (browserFieldValue !== undefined) return;

// This is here to support consumers who have used the
// "external" option to exclude all Node builtins (e.g.
// Remix v1 does this), otherwise the import/require is left
// in the output and throws an error at runtime. Ideally we
// would just return undefined for any browser field value,
// and we can safely switch to this in a major version.
if (browserFieldValue === false) {
return emptyResult;
}

if (browserFieldValue !== undefined) {
return;
}
}

const moduleName = normalizeNodeBuiltinPath(args.path);
Expand Down
5 changes: 3 additions & 2 deletions tests/scenarios/__snapshots__/browserFieldFalse.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ exports[`Browser Field False Test > GIVEN a file in a browser target build that
mod
));
// (disabled):constants
// node-modules-polyfills-empty:constants
var require_constants = __commonJS({
\\"(disabled):constants\\"() {
\\"node-modules-polyfills-empty:constants\\"(exports, module) {
module.exports = {};
}
});
Expand Down

0 comments on commit 3946db8

Please sign in to comment.