Skip to content

Commit

Permalink
fix: Handle the scenario where a package.json#browser field could be …
Browse files Browse the repository at this point in the history
…`false` (#427)

We [recently shipped](#424) support
for the `browser` field, but we discovered an edge case where instead of
mapping to another string, the value could be `false` (eg,
[`object-inspect`](https://github.com/inspect-js/object-inspect/blob/main/package.json#L82)),
which indicates that this module should be treated as an empty object
(`{}`) (evidenced by its usage
[here](https://github.com/inspect-js/object-inspect/blob/main/index.js#L68-L70)).

The code was previously erroring on the string's `startsWith` checks,
causing the entire package to be omitted from the file mapping result.
This ensures that if the value is `false` we'll continue on.

---------

Co-authored-by: Steven <steven@ceriously.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed Jul 11, 2024
1 parent 7ba8042 commit 099608f
Show file tree
Hide file tree
Showing 26 changed files with 93 additions and 2 deletions.
12 changes: 11 additions & 1 deletion src/resolve-dependency.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ interface PkgCfg {
main: string | undefined;
exports: PackageTarget;
imports: { [key: string]: PackageTarget };
browser?: string | { [key: string]: string };
browser?: unknown;
}

async function getPkgCfg(
Expand Down Expand Up @@ -259,8 +259,18 @@ async function resolveRemappings(
): Promise<void> {
if (job.conditions?.includes('browser')) {
const { browser: pkgBrowser } = pkgCfg;
if (!pkgBrowser) {
return;
}
if (typeof pkgBrowser === 'object') {
for (const [key, value] of Object.entries(pkgBrowser)) {
if (typeof value !== 'string') {
/**
* `false` can be used to specify that a file is not meant to be included.
* Downstream processing is expected to handle this case, and it should remain in the mapping result
*/
continue;
}
if (!key.startsWith('./') || !value.startsWith('./')) {
continue;
}
Expand Down
2 changes: 2 additions & 0 deletions test/unit/browser-remappings-false/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# include node_modules for testing
!node_modules
2 changes: 2 additions & 0 deletions test/unit/browser-remappings-false/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
require('pkg');

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions test/unit/browser-remappings-false/node_modules/pkg/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions test/unit/browser-remappings-false/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
"package.json",
"test/unit/browser-remappings-false/input.js",
"test/unit/browser-remappings-false/node_modules/pkg/browser.js",
"test/unit/browser-remappings-false/node_modules/pkg/index.js",
"test/unit/browser-remappings-false/node_modules/pkg/package.json"
]
3 changes: 3 additions & 0 deletions test/unit/browser-remappings-false/test-opts.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"conditions": ["browser"]
}
2 changes: 2 additions & 0 deletions test/unit/browser-remappings-string/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# include node_modules for testing
!node_modules
2 changes: 2 additions & 0 deletions test/unit/browser-remappings-string/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
require('pkg');

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions test/unit/browser-remappings-string/node_modules/pkg/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions test/unit/browser-remappings-string/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
"package.json",
"test/unit/browser-remappings-string/input.js",
"test/unit/browser-remappings-string/node_modules/pkg/browser.js",
"test/unit/browser-remappings-string/node_modules/pkg/index.js",
"test/unit/browser-remappings-string/node_modules/pkg/package.json"
]
3 changes: 3 additions & 0 deletions test/unit/browser-remappings-string/test-opts.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"conditions": ["browser"]
}
2 changes: 2 additions & 0 deletions test/unit/browser-remappings-undefined/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# include node_modules for testing
!node_modules
2 changes: 2 additions & 0 deletions test/unit/browser-remappings-undefined/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
require('pkg');

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions test/unit/browser-remappings-undefined/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[
"package.json",
"test/unit/browser-remappings-undefined/input.js",
"test/unit/browser-remappings-undefined/node_modules/pkg/index.js",
"test/unit/browser-remappings-undefined/node_modules/pkg/package.json",
"test/unit/browser-remappings-undefined/node_modules/pkg/require-main.cjs",
"test/unit/browser-remappings-undefined/node_modules/pkg/subdir/import-main.js",
"test/unit/browser-remappings-undefined/node_modules/pkg/subdir/package.json"
]
3 changes: 3 additions & 0 deletions test/unit/browser-remappings-undefined/test-opts.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"conditions": ["browser"]
}
2 changes: 1 addition & 1 deletion test/unit/browser-remappings/output.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@
"test/unit/browser-remappings/node_modules/pkg/subdir/import-main-browser.js",
"test/unit/browser-remappings/node_modules/pkg/subdir/import-main.js",
"test/unit/browser-remappings/node_modules/pkg/subdir/package.json"
]
]

0 comments on commit 099608f

Please sign in to comment.