Skip to content

Commit

Permalink
Warn on exports browser-ish conflict
Browse files Browse the repository at this point in the history
closes #58
  • Loading branch information
bluwy committed Sep 28, 2023
1 parent c3fff13 commit 160318f
Show file tree
Hide file tree
Showing 11 changed files with 114 additions and 18 deletions.
11 changes: 11 additions & 0 deletions pkg/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,17 @@ export type Message =
expectTypes: string[]
}
>
| BaseMessage<
'EXPORTS_VALUE_CONFLICTS_WITH_BROWSER',
{
/**
* The path to the key inside the `"browser"` field that conflicts with
* the current path `"exports"` field
*/
browserPath: string[]
browserishCondition: string
}
>

export interface Options {
/**
Expand Down
25 changes: 25 additions & 0 deletions pkg/src/constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Some exports condition are known to be similar to the browser condition but
// runs slightly different. Specifically, the `pkg.browser` field will be applied
// even after resolving with these export conditions, which can be confusing at times.
export const knownBrowserishConditions = [
'worker',
// Cloudflare Workers: https://runtime-keys.proposal.wintercg.org/#workerd
'workerd',
// Vercel Edge: https://runtime-keys.proposal.wintercg.org/#edge-light
'edge-light'
]

// NOTE: this list is intentionally non-exhaustive and subjective as it's used for
// loose detection purpose for `pkg.files` recommendation only. please dont submit
// a PR to extend it.
export const commonInternalPaths = [
// directories
'test/',
'tests/',
'__tests__/',
// files
'.prettierrc',
'prettier.config.js',
'.eslintrc',
'.eslintrc.js'
]
24 changes: 22 additions & 2 deletions pkg/src/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { commonInternalPaths, knownBrowserishConditions } from './constants.js'
import {
exportsGlob,
getCodeFormat,
Expand All @@ -15,8 +16,7 @@ import {
getDtsFilePathFormat,
getDtsCodeFormatExtension,
getPkgPathValue,
replaceLast,
commonInternalPaths
replaceLast
} from './utils.js'

/**
Expand Down Expand Up @@ -490,6 +490,26 @@ export async function publint({ pkgDir, vfs, level, strict, _packedFiles }) {
return
}

// if the exports value matches a key in `pkg.browser` (meaning it'll be remapped
// if in a browser-ish environment), check if this is a browser-ish environment/condition.
// if so, warn about this conflict as it's often unexpected behaviour.
if (typeof browser === 'object' && exportsValue in browser) {
const browserishCondition = knownBrowserishConditions.find((c) =>
currentPath.includes(c)
)
if (browserishCondition) {
messages.push({
code: 'EXPORTS_VALUE_CONFLICTS_WITH_BROWSER',
args: {
browserPath: browserPkgPath.concat(exportsValue),
browserishCondition
},
path: currentPath,
type: 'warning'
})
}
}

const pq = createPromiseQueue()

// TODO: group glob warnings
Expand Down
3 changes: 3 additions & 0 deletions pkg/src/message.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ export function formatMessage(m, pkg) {
// prettier-ignore
return `${c.bold(fp(m.path))} is ${c.bold(pv(m.path))} which is an invalid ${c.bold(m.args.actualType)} type. Expected a ${c.bold(expectStr)} type instead.`
}
case 'EXPORTS_VALUE_CONFLICTS_WITH_BROWSER':
// prettier-ignore
return `${c.bold(fp(m.path))} is ${c.bold(pv(m.path))} which also matches ${c.bold(fp(m.args.browserPath))}: "${c.bold(pv(m.args.browserPath))}", which overrides the path when building the library with the "${c.bold(m.args.browserishCondition)}" condition. This is usually unintentional and may cause build issues. Consider using a different file name for ${c.bold(pv(m.path))}.`
default:
return
}
Expand Down
15 changes: 0 additions & 15 deletions pkg/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,6 @@
* }} Pkg
*/

// NOTE: this list is intentionally non-exhaustive and subjective as it's used for
// loose detection purpose for `pkg.files` recommendation only. please dont submit
// a PR to extend it.
export const commonInternalPaths = [
// directories
'test/',
'tests/',
'__tests__/',
// files
'.prettierrc',
'prettier.config.js',
'.eslintrc',
'.eslintrc.js'
]

/**
* @typedef {'ESM' | 'CJS' | 'mixed' | 'unknown'} CodeFormat
*/
Expand Down
Empty file.
Empty file.
19 changes: 19 additions & 0 deletions pkg/tests/fixtures/exports-browser-conflict/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"name": "publint-exports-browser-conflict",
"version": "0.0.1",
"private": true,
"browser": {
"./lib.server.js": "./lib.browser.js"
},
"exports": {
".": {
"worker": {
"import": "./lib.server.js"
},
"browser": {
"import": "./lib.browser.js"
},
"default": "./lib.server.js"
}
}
}
5 changes: 5 additions & 0 deletions pkg/tests/playground.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ import { equal } from 'uvu/assert'
import { publint } from '../lib/node.js'
import { formatMessage } from '../lib/utils-node.js'

testFixture('exports-browser-conflict', [
'EXPORTS_VALUE_CONFLICTS_WITH_BROWSER',
'USE_EXPORTS_OR_IMPORTS_BROWSER'
])

testFixture('exports-styles', [
'EXPORTS_VALUE_INVALID',
'FILE_DOES_NOT_EXIST',
Expand Down
25 changes: 25 additions & 0 deletions site/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,28 @@ Internal tests or config files are published, which are usually not needed and u
## `FIELD_INVALID_VALUE_TYPE`

Some `package.json` fields has a set of allowed types, e.g. `string` or `object` only. If an invalid type is passed, this error message will be showed.

## `EXPORTS_VALUE_CONFLICTS_WITH_BROWSER`

When an `"exports"` value resolved with a browser-ish condition matches a key in the `"browser"` field object, this means the `"exports"` value is overriden by that matching `"browser"` key. This may cause build issues as the intended `"exports"` value is no longer used. For example, given this setup:

```json
{
"browser": {
"./lib.server.js": "./lib.browser.js"
},
"exports": {
".": {
"worker": "./lib.server.js",
"browser": "./lib.browser.js",
"default": "./lib.server.js"
}
}
}
```

When matching the `"worker"` condition, it will resolve to `"./lib.server.js"` which is intended to work in a worker environment. However, the `"browser"` field also has a matching mapping for `"./lib.server.js"`, causing the final resolved path to be `"./lib.browser.js"`.

This is usually not intended and causes the wrong file to be loaded. If it is intended, the `"worker"` condition should point to `"./lib.browser.js"` directly instead.

To fix this, you can rename `"./lib.server.js"` to `"./lib.worker.js"` for example so it has its own specific file. Or check out the [USE_EXPORTS_OR_IMPORTS_BROWSER](#use_exports_or_imports_browser) rule to refactor away the `"browser"` field.
5 changes: 4 additions & 1 deletion site/src/utils/message.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,11 @@ function messageToString(m, pkg) {
}
}
// prettier-ignore
return `${bold(fp(m.path))} is ${bold(pv(m.path))} which is an invalid ${bold(m.args.actualType)} type. Expected a ${bold(expectStr)} type instead.`
return `The field value has an invalid ${bold(m.args.actualType)} type. Expected a ${bold(expectStr)} type instead.`
}
case 'EXPORTS_VALUE_CONFLICTS_WITH_BROWSER':
// prettier-ignore
return `${bold(pv(m.path))} matches ${bold(fp(m.args.browserPath))}: "${bold(pv(m.args.browserPath))}", which overrides the path when building the library with the "${bold(m.args.browserishCondition)}" condition. This is usually unintentional and may cause build issues. Consider using a different file name for ${bold(pv(m.path))}.`
default:
return
}
Expand Down

0 comments on commit 160318f

Please sign in to comment.