Skip to content

Commit

Permalink
Improve browser field suggestion (#59)
Browse files Browse the repository at this point in the history
  • Loading branch information
sapphi-red authored Aug 20, 2023
1 parent f5e8c9a commit cff41dd
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 15 deletions.
1 change: 1 addition & 0 deletions pkg/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export type Message =
| BaseMessage<'EXPORTS_MODULE_SHOULD_BE_ESM'>
| BaseMessage<'EXPORTS_VALUE_INVALID', { suggestValue: string }>
| BaseMessage<'USE_EXPORTS_BROWSER'>
| BaseMessage<'USE_EXPORTS_OR_IMPORTS_BROWSER'>
| BaseMessage<
'TYPES_NOT_EXPORTED',
{
Expand Down
21 changes: 15 additions & 6 deletions pkg/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,21 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) {
// if the package has both the `browser` and `exports` fields, recommend to use
// the browser condition instead
if (exports) {
messages.push({
code: 'USE_EXPORTS_BROWSER',
args: {},
path: browserPkgPath,
type: 'suggestion'
})
if (typeof browser === 'string') {
messages.push({
code: 'USE_EXPORTS_BROWSER',
args: {},
path: browserPkgPath,
type: 'suggestion'
})
} else {
messages.push({
code: 'USE_EXPORTS_OR_IMPORTS_BROWSER',
args: {},
path: browserPkgPath,
type: 'suggestion'
})
}
}
}

Expand Down
6 changes: 5 additions & 1 deletion pkg/src/message.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ export function formatMessage(m, pkg) {
return `${c.bold(fp(m.path))} is ${c.bold(pv(m.path))} but is invalid as it does not start with "${c.bold('./')}". Use ${c.bold(m.args.suggestValue)} instead.`
case 'USE_EXPORTS_BROWSER':
// prettier-ignore
return `${c.bold('pkg.browser')} can be refactored to use ${c.bold('pkg.exports')} and the ${c.bold('"browser"')} condition instead to declare browser-specific exports. (This will be a breaking change)`
return `${c.bold('pkg.browser')} with a string value can be refactored to use ${c.bold('pkg.exports')} and the ${c.bold('"browser"')} condition to declare browser-specific exports. ` +
`e.g. ${c.bold('pkg.exports["."].browser')}: "${c.bold(pv(m.path))}". (This will be a breaking change)`
case 'USE_EXPORTS_OR_IMPORTS_BROWSER':
// prettier-ignore
return `${c.bold('pkg.browser')} with an object value can be refactored to use ${c.bold('pkg.exports')}/${c.bold('pkg.imports')} and the ${c.bold('"browser"')} condition to declare browser-specific exports. (This will be a breaking change)`
case 'TYPES_NOT_EXPORTED': {
const typesFilePath = exportsRel(m.args.typesFilePath)
if (m.args.actualExtension && m.args.expectExtension) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/tests/playground.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ testFixture('glob-deprecated', [
testFixture('missing-files', [
...Array(7).fill('FILE_DOES_NOT_EXIST'),
'FILE_NOT_PUBLISHED',
'USE_EXPORTS_BROWSER'
'USE_EXPORTS_OR_IMPORTS_BROWSER'
])

testFixture('no-exports-module', [])
Expand Down
58 changes: 52 additions & 6 deletions site/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,58 @@ The `"exports"` field value should always start with a `./`. It does not support

## `USE_EXPORTS_BROWSER`

The `"browser"` field and `"exports"` `"browser"` condition works similarly to define the browser counterpart of a package. With the overlap, it's usually better to use the `"exports"` field instead as it's widely supported, and keeps one true way of defining your package entrypoints.
A `"browser"` field with a string value works similarly to the `"exports"` `"browser"` condition, to define the browser-specific exports of a package. Between the two, it's usually better to use the `"exports"` field instead as it's standardized, widely supported, and keeps one true way of defining your package entrypoints.

It's important to note that the `"browser"` field can be used for [many more cases](https://github.com/defunctzombie/package-browser-field-spec), like:
## `USE_EXPORTS_OR_IMPORTS_BROWSER`

- Replace the root entrypoint with a browser-safe file.
- Replace a nested file or module specifier with a browser-safe file.
- Ignore a specific module specifier so it's not loaded.
The `"browser"` field with an object value works similarly to the `"exports"`/`"imports"` `"browser"` condition, to define the browser-specific exports of a package. Between the two, it's usually better to use the `"exports"`/`"imports"` field instead as it's standardized, widely supported, and keeps one true way of defining your package entrypoints.

So switching to the `"exports"` field may not be a one-to-one migration, and that's fine! Once you're ready to restructure your code, you can make these changes going forward.
For example, the following `"browser"` field can be converted like below.

Before:

```json
{
"browser": {
"module-a": "./shims/module-a.js",
"module-b": false,
"./server/only.js": "./shims/client-only.js"
}
}
```

After:

```json
{
"imports": {
"#module-a": {
"browser": "./shims/module-a.js",
"default": "module-a"
},
"#module-b": {
"browser": "./empty.js",
"default": "module-b"
},
"#server-only.js": {
"browser": "./shims/client-only.js",
"default": "./server/only.js"
}
}
}
```

Note that you'll need to change all imports to use the specifier defined in the `"imports"` field. For example, `import foo from "module-a"` -> `import foo from "#module-a"`.

Depending on your setup, you can also use the `"exports"` field to directly export the browser-specific entrypoint. For example:

```json
{
"exports": {
".": {
"browser": "./lib.browser.js",
"default": "./lib.js"
}
}
}
```
6 changes: 5 additions & 1 deletion site/src/utils/message.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,11 @@ function messageToString(m, pkg) {
return `${bold(pv(m.path))} is invalid as it does not start with "${bold('./')}". Use ${bold(m.args.suggestValue)} instead.`
case 'USE_EXPORTS_BROWSER':
// prettier-ignore
return `${bold('pkg.browser')} can be refactored to use ${bold('pkg.exports')} and the ${bold('"browser"')} condition instead to declare browser-specific exports. (This will be a breaking change)`
return `${bold('pkg.browser')} with a string value can be refactored to use ${bold('pkg.exports')} and the ${bold('"browser"')} condition to declare browser-specific exports. `
+ `e.g. ${bold('pkg.exports["."].browser')}: "${bold(pv(m.path))}". (This will be a breaking change)`
case 'USE_EXPORTS_OR_IMPORTS_BROWSER':
// prettier-ignore
return `${bold('pkg.browser')} with an object value can be refactored to use ${bold('pkg.exports')}/${bold('pkg.imports')} and the ${bold('"browser"')} condition to declare browser-specific exports. (This will be a breaking change)`
case 'TYPES_NOT_EXPORTED': {
const typesFilePath = exportsRel(m.args.typesFilePath)
if (m.args.actualExtension && m.args.expectExtension) {
Expand Down

0 comments on commit cff41dd

Please sign in to comment.