Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ type ModuleOptions = {
requireMainStrategy?: 'import-meta-main' | 'realpath'
detectCircularRequires?: 'off' | 'warn' | 'error'
detectDualPackageHazard?: 'off' | 'warn' | 'error'
dualPackageHazardScope?: 'file' | 'project'
requireSource?: 'builtin' | 'create-require'
importMetaPrelude?: 'off' | 'auto' | 'on'
cjsDefault?: 'module-exports' | 'auto' | 'none'
Expand All @@ -156,7 +157,8 @@ type ModuleOptions = {
- `requireMainStrategy` (`import-meta-main`): use `import.meta.main` or the realpath-based `pathToFileURL(realpathSync(process.argv[1])).href` check.
- `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.
- `detectCircularRequires` (`off`): optionally detect relative static require cycles and warn/throw.
- `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.
- `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.
- `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.
- `topLevelAwait` (`error`): throw, wrap, or preserve when TLA appears in CommonJS output.
- `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.
- `requireSource` (`builtin`): whether `require` comes from Node or `createRequire`.
Expand Down
1 change: 1 addition & 0 deletions docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Short and long forms are supported.
| -i | --append-directory-index | Append directory index (e.g. index.js) or false |
| -c | --detect-circular-requires | Warn/error on circular require (off \| warn \| error) |
| -H | --detect-dual-package-hazard | Warn/error on mixed import/require of dual packages (off \| warn \| error) |
| | --dual-package-hazard-scope | Scope for dual package hazard detection (file \| project) |
| -a | --top-level-await | TLA handling (error \| wrap \| preserve) |
| -d | --cjs-default | Default interop (module-exports \| auto \| none) |
| -e | --idiomatic-exports | Emit idiomatic exports when safe (off \| safe \| aggressive) |
Expand Down
12 changes: 11 additions & 1 deletion docs/dual-package-hazard.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ This tool can warn or error when a file mixes specifiers that may trigger the du
- `warn`: emit diagnostics but continue.
- `error`: diagnostics are emitted and the transform exits non-zero.
- `off`: skip detection.
- `dualPackageHazardScope`: `file` (default) | `project`
- CLI: `--dual-package-hazard-scope` (long-only).
- `file`: run detection independently per file (legacy behavior).
- `project`: aggregate usages across all CLI inputs, then emit one diagnostic set per package. Per-file detection is disabled when this is on.

## What we detect (per file)

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

## What is not covered

- Cross-file or whole-project graph analysis; detection is per file only.
- Cross-file or whole-project graph analysis unless `dualPackageHazardScope: 'project'` is enabled.
- Dynamic or template specifiers; non-literal specifiers are ignored.
- Loader/bundler resolution differences (pnpm linking, aliases, custom conditions).
- 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.

## Project-wide analysis (opt-in)

- Set `--dual-package-hazard-scope project` (CLI) or `dualPackageHazardScope: 'project'` (API).
- 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.
- Still uses static literal specifiers and manifest reads under `node_modules`; aliasing/path-mapping differences may not be reflected.

## Guidance

- 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.
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

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

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@knighted/module",
"version": "1.4.0-rc.0",
"version": "1.4.0-rc.1",
"description": "Bidirectional transform for ES modules and CommonJS.",
"type": "module",
"main": "dist/module.js",
Expand Down
38 changes: 34 additions & 4 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { dirname, resolve, relative, join } from 'node:path'
import { builtinModules } from 'node:module'
import { glob } from 'glob'

import { transform } from './module.js'
import { transform, collectProjectDualPackageHazards } from './module.js'
import { parse } from './parse.js'
import { format } from './format.js'
import { specifier } from './specifier.js'
Expand All @@ -31,6 +31,7 @@ const defaultOptions: ModuleOptions = {
requireMainStrategy: 'import-meta-main',
detectCircularRequires: 'off',
detectDualPackageHazard: 'warn',
dualPackageHazardScope: 'file',
requireSource: 'builtin',
nestedRequireStrategy: 'create-require',
cjsDefault: 'auto',
Expand Down Expand Up @@ -218,6 +219,12 @@ const optionsTable = [
type: 'string',
desc: 'Warn/error on mixed import/require of dual packages (off|warn|error)',
},
{
long: 'dual-package-hazard-scope',
short: undefined,
type: 'string',
desc: 'Scope for dual package hazard detection (file|project)',
},
{
long: 'top-level-await',
short: 'a',
Expand Down Expand Up @@ -315,7 +322,9 @@ type ParsedValues = Parsed['values']
const buildHelp = (enableColor: boolean) => {
const c = colorize(enableColor)
const maxFlagLength = Math.max(
...optionsTable.map(opt => ` -${opt.short}, --${opt.long}`.length),
...optionsTable.map(opt =>
opt.short ? ` -${opt.short}, --${opt.long}`.length : ` --${opt.long}`.length,
),
)
const lines = [
`${c.bold('Usage:')} dub [options] <files...>`,
Expand All @@ -329,7 +338,7 @@ const buildHelp = (enableColor: boolean) => {
]

for (const opt of optionsTable) {
const flag = ` -${opt.short}, --${opt.long}`
const flag = opt.short ? ` -${opt.short}, --${opt.long}` : ` --${opt.long}`
const pad = ' '.repeat(Math.max(2, maxFlagLength - flag.length + 2))
lines.push(`${c.bold(flag)}${pad}${opt.desc}`)
}
Expand Down Expand Up @@ -394,6 +403,11 @@ const toModuleOptions = (values: ParsedValues): ModuleOptions => {
values['detect-dual-package-hazard'] as string | undefined,
['off', 'warn', 'error'] as const,
) ?? defaultOptions.detectDualPackageHazard,
dualPackageHazardScope:
parseEnum(
values['dual-package-hazard-scope'] as string | undefined,
['file', 'project'] as const,
) ?? defaultOptions.dualPackageHazardScope,
topLevelAwait:
parseEnum(
values['top-level-await'] as string | undefined,
Expand Down Expand Up @@ -555,6 +569,12 @@ const runFiles = async (
) => {
const results: FileResult[] = []
const logger = makeLogger(io.stdout, io.stderr)
const hazardScope = moduleOpts.dualPackageHazardScope ?? 'file'
const hazardMode = moduleOpts.detectDualPackageHazard ?? 'warn'
const projectHazards =
hazardScope === 'project' && hazardMode !== 'off'
? await collectProjectDualPackageHazards(files, moduleOpts)
: null

for (const file of files) {
const diagnostics: Diagnostic[] = []
Expand All @@ -568,6 +588,8 @@ const runFiles = async (
out: undefined,
inPlace: false,
filePath: file,
detectDualPackageHazard:
hazardScope === 'project' ? 'off' : moduleOpts.detectDualPackageHazard,
}

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

if (projectHazards) {
const extras = projectHazards.get(file)
if (extras?.length) diagnostics.push(...extras)
}

if (flags.list && changed) {
logger.info(file)
}
Expand Down Expand Up @@ -631,7 +658,10 @@ const runCli = async ({
options: Object.fromEntries(
optionsTable.map(opt => [
opt.long,
{ type: opt.type as 'string' | 'boolean', short: opt.short },
{
type: opt.type as 'string' | 'boolean',
...(opt.short ? { short: opt.short } : {}),
},
]),
),
})
Expand Down
Loading