Skip to content

Commit 2f8e2f1

Browse files
feat: project wide dual hazard diagnostics. (#42)
1 parent 0e29901 commit 2f8e2f1

File tree

10 files changed

+446
-82
lines changed

10 files changed

+446
-82
lines changed

README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ type ModuleOptions = {
131131
requireMainStrategy?: 'import-meta-main' | 'realpath'
132132
detectCircularRequires?: 'off' | 'warn' | 'error'
133133
detectDualPackageHazard?: 'off' | 'warn' | 'error'
134+
dualPackageHazardScope?: 'file' | 'project'
134135
requireSource?: 'builtin' | 'create-require'
135136
importMetaPrelude?: 'off' | 'auto' | 'on'
136137
cjsDefault?: 'module-exports' | 'auto' | 'none'
@@ -156,7 +157,8 @@ type ModuleOptions = {
156157
- `requireMainStrategy` (`import-meta-main`): use `import.meta.main` or the realpath-based `pathToFileURL(realpathSync(process.argv[1])).href` check.
157158
- `importMetaPrelude` (`auto`): emit a no-op `void import.meta.filename;` touch. `on` always emits; `off` never emits; `auto` emits only when helpers that reference `import.meta.*` are synthesized (e.g., `__dirname`/`__filename` in CJS→ESM, require-main shims, createRequire helpers). Useful for bundlers/transpilers that do usage-based `import.meta` polyfilling.
158159
- `detectCircularRequires` (`off`): optionally detect relative static require cycles and warn/throw.
159-
- `detectDualPackageHazard` (`warn`): flag when a file mixes `import` and `require` of the same package or combines root and subpath specifiers that can resolve to separate module instances (dual packages). Set to `error` to fail the transform.
160+
- `detectDualPackageHazard` (`warn`): flag when `import` and `require` mix for the same package or root/subpath are combined in ways that can resolve to separate module instances (dual packages). Set to `error` to fail the transform.
161+
- `dualPackageHazardScope` (`file`): `file` preserves the legacy per-file detector; `project` aggregates package usage across all CLI inputs (useful in monorepos/hoisted installs) and emits one diagnostic per package.
160162
- `topLevelAwait` (`error`): throw, wrap, or preserve when TLA appears in CommonJS output.
161163
- `rewriteSpecifier` (off): rewrite relative specifiers to a chosen extension or via a callback. Precedence: the callback (if provided) runs first; if it returns a string, that wins. If it returns `undefined` or `null`, the appenders still apply.
162164
- `requireSource` (`builtin`): whether `require` comes from Node or `createRequire`.

docs/cli.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ Short and long forms are supported.
7070
| -i | --append-directory-index | Append directory index (e.g. index.js) or false |
7171
| -c | --detect-circular-requires | Warn/error on circular require (off \| warn \| error) |
7272
| -H | --detect-dual-package-hazard | Warn/error on mixed import/require of dual packages (off \| warn \| error) |
73+
| | --dual-package-hazard-scope | Scope for dual package hazard detection (file \| project) |
7374
| -a | --top-level-await | TLA handling (error \| wrap \| preserve) |
7475
| -d | --cjs-default | Default interop (module-exports \| auto \| none) |
7576
| -e | --idiomatic-exports | Emit idiomatic exports when safe (off \| safe \| aggressive) |

docs/dual-package-hazard.md

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ This tool can warn or error when a file mixes specifiers that may trigger the du
99
- `warn`: emit diagnostics but continue.
1010
- `error`: diagnostics are emitted and the transform exits non-zero.
1111
- `off`: skip detection.
12+
- `dualPackageHazardScope`: `file` (default) | `project`
13+
- CLI: `--dual-package-hazard-scope` (long-only).
14+
- `file`: run detection independently per file (legacy behavior).
15+
- `project`: aggregate usages across all CLI inputs, then emit one diagnostic set per package. Per-file detection is disabled when this is on.
1216

1317
## What we detect (per file)
1418

@@ -28,11 +32,17 @@ This tool can warn or error when a file mixes specifiers that may trigger the du
2832

2933
## What is not covered
3034

31-
- Cross-file or whole-project graph analysis; detection is per file only.
35+
- Cross-file or whole-project graph analysis unless `dualPackageHazardScope: 'project'` is enabled.
3236
- Dynamic or template specifiers; non-literal specifiers are ignored.
3337
- Loader/bundler resolution differences (pnpm linking, aliases, custom conditions).
3438
- Exact equality of root vs subpath targets; we do not stat/resolve to see if they point to the same file, so a root/subpath warning may be conservative.
3539

40+
## Project-wide analysis (opt-in)
41+
42+
- Set `--dual-package-hazard-scope project` (CLI) or `dualPackageHazardScope: 'project'` (API).
43+
- The CLI pre-scans all input files, aggregates package usage (import vs require, root vs subpath), and emits diagnostics per package. Per-file hazard checks are turned off in this mode to avoid duplicate messages.
44+
- Still uses static literal specifiers and manifest reads under `node_modules`; aliasing/path-mapping differences may not be reflected.
45+
3646
## Guidance
3747

3848
- Prefer a single specifier form for a given package: either all import or all require, and avoid mixing root and subpath unless you know they share the same build.

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@knighted/module",
3-
"version": "1.4.0-rc.0",
3+
"version": "1.4.0-rc.1",
44
"description": "Bidirectional transform for ES modules and CommonJS.",
55
"type": "module",
66
"main": "dist/module.js",

src/cli.ts

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { dirname, resolve, relative, join } from 'node:path'
1010
import { builtinModules } from 'node:module'
1111
import { glob } from 'glob'
1212

13-
import { transform } from './module.js'
13+
import { transform, collectProjectDualPackageHazards } from './module.js'
1414
import { parse } from './parse.js'
1515
import { format } from './format.js'
1616
import { specifier } from './specifier.js'
@@ -31,6 +31,7 @@ const defaultOptions: ModuleOptions = {
3131
requireMainStrategy: 'import-meta-main',
3232
detectCircularRequires: 'off',
3333
detectDualPackageHazard: 'warn',
34+
dualPackageHazardScope: 'file',
3435
requireSource: 'builtin',
3536
nestedRequireStrategy: 'create-require',
3637
cjsDefault: 'auto',
@@ -218,6 +219,12 @@ const optionsTable = [
218219
type: 'string',
219220
desc: 'Warn/error on mixed import/require of dual packages (off|warn|error)',
220221
},
222+
{
223+
long: 'dual-package-hazard-scope',
224+
short: undefined,
225+
type: 'string',
226+
desc: 'Scope for dual package hazard detection (file|project)',
227+
},
221228
{
222229
long: 'top-level-await',
223230
short: 'a',
@@ -315,7 +322,9 @@ type ParsedValues = Parsed['values']
315322
const buildHelp = (enableColor: boolean) => {
316323
const c = colorize(enableColor)
317324
const maxFlagLength = Math.max(
318-
...optionsTable.map(opt => ` -${opt.short}, --${opt.long}`.length),
325+
...optionsTable.map(opt =>
326+
opt.short ? ` -${opt.short}, --${opt.long}`.length : ` --${opt.long}`.length,
327+
),
319328
)
320329
const lines = [
321330
`${c.bold('Usage:')} dub [options] <files...>`,
@@ -329,7 +338,7 @@ const buildHelp = (enableColor: boolean) => {
329338
]
330339

331340
for (const opt of optionsTable) {
332-
const flag = ` -${opt.short}, --${opt.long}`
341+
const flag = opt.short ? ` -${opt.short}, --${opt.long}` : ` --${opt.long}`
333342
const pad = ' '.repeat(Math.max(2, maxFlagLength - flag.length + 2))
334343
lines.push(`${c.bold(flag)}${pad}${opt.desc}`)
335344
}
@@ -394,6 +403,11 @@ const toModuleOptions = (values: ParsedValues): ModuleOptions => {
394403
values['detect-dual-package-hazard'] as string | undefined,
395404
['off', 'warn', 'error'] as const,
396405
) ?? defaultOptions.detectDualPackageHazard,
406+
dualPackageHazardScope:
407+
parseEnum(
408+
values['dual-package-hazard-scope'] as string | undefined,
409+
['file', 'project'] as const,
410+
) ?? defaultOptions.dualPackageHazardScope,
397411
topLevelAwait:
398412
parseEnum(
399413
values['top-level-await'] as string | undefined,
@@ -555,6 +569,12 @@ const runFiles = async (
555569
) => {
556570
const results: FileResult[] = []
557571
const logger = makeLogger(io.stdout, io.stderr)
572+
const hazardScope = moduleOpts.dualPackageHazardScope ?? 'file'
573+
const hazardMode = moduleOpts.detectDualPackageHazard ?? 'warn'
574+
const projectHazards =
575+
hazardScope === 'project' && hazardMode !== 'off'
576+
? await collectProjectDualPackageHazards(files, moduleOpts)
577+
: null
558578

559579
for (const file of files) {
560580
const diagnostics: Diagnostic[] = []
@@ -568,6 +588,8 @@ const runFiles = async (
568588
out: undefined,
569589
inPlace: false,
570590
filePath: file,
591+
detectDualPackageHazard:
592+
hazardScope === 'project' ? 'off' : moduleOpts.detectDualPackageHazard,
571593
}
572594

573595
let writeTarget: string | undefined
@@ -587,6 +609,11 @@ const runFiles = async (
587609
const output = await transform(file, perFileOpts)
588610
const changed = output !== original
589611

612+
if (projectHazards) {
613+
const extras = projectHazards.get(file)
614+
if (extras?.length) diagnostics.push(...extras)
615+
}
616+
590617
if (flags.list && changed) {
591618
logger.info(file)
592619
}
@@ -631,7 +658,10 @@ const runCli = async ({
631658
options: Object.fromEntries(
632659
optionsTable.map(opt => [
633660
opt.long,
634-
{ type: opt.type as 'string' | 'boolean', short: opt.short },
661+
{
662+
type: opt.type as 'string' | 'boolean',
663+
...(opt.short ? { short: opt.short } : {}),
664+
},
635665
]),
636666
),
637667
})

0 commit comments

Comments
 (0)