Skip to content

feat(config): accept CLI-only build flags in pkg config#263

Open
robertsLando wants to merge 7 commits intomainfrom
refactor/config-pipeline
Open

feat(config): accept CLI-only build flags in pkg config#263
robertsLando wants to merge 7 commits intomainfrom
refactor/config-pipeline

Conversation

@robertsLando
Copy link
Copy Markdown
Member

@robertsLando robertsLando commented Apr 23, 2026

Closes #262.

Summary

Every CLI build-shaping flag is now also accepted in the pkg config file (.pkgrc, pkg.config.{js,cjs,mjs,json}, or package.json#pkg), so tooling (GitHub Action, IDE plugins, CI wrappers) can drive the build from a single declarative source instead of synthesizing a CLI invocation.

Newly accepted config keys

CLI flag Config key Type
--compress compress 'None' | 'Brotli' | 'GZip' | 'Zstd'
--fallback-to-source fallbackToSource boolean
--public public boolean
--public-packages publicPackages string | string[]
--options options string | string[]
--no-bytecode bytecode: false boolean
--no-native-build nativeBuild: false boolean
--no-dict noDictionary string | string[]
--debug debug boolean
--signature signature boolean
  • CLI flags override config values (matches existing targets / outputPath convention).
  • Unknown keys produce a warning; wrong types throw a clear error.

Under the hood

The pipeline that threads these values from CLI + config into the build got rewritten to support the single-source-of-truth direction:

  • Single FLAG_SPECS registry — one table maps each flag's CLI name, config key, resolved key, kind, and default. parseArgs options, --no-* siblings, config validator, option→argv mapping, and CLI>config>default merge are all derived from this list, so adding a new flag means editing one place.
  • Canonical ParsedInput — both CLI (string[]) and programmatic (PkgExecOptions) entry points produce the same shape. Drops the options→argv→parseArgs round trip.
  • Phase-split resolveConfig — named helpers: resolveInput / resolveConfigFile / resolveOutput / resolveTargetList / assignTargetOutputs. resolveConfig is ~30 lines of orchestration.
  • Fully resolved flagscompress is CompressType enum, targets is NodeTarget[] with per-target output paths and input-overwrite guards applied. Effective merged pkg is exposed on ResolvedConfig so index.ts no longer duplicates precedence.
  • Moved parseTargets / stringifyTarget / differentParts / stringifyTargetForOutput into config.ts alongside the rest of the resolution logic.
  • Dropped unused minimist dep (CLI already on util.parseArgs).

exec() sheds ~80 lines of post-resolve logic.

Test plan

  • yarn build
  • yarn lint
  • New test/test-50-config-flags/ — exercises config file driving the full flag surface, CLI override (--options "" wins over configured list), bad type rejection, unknown-key warning
  • node test/test.js host no-npm "test-46-input*" — 28/28 (input/output/target matrix)
  • node test/test.js host no-npm "test-50-*" — 96/96
  • node test/test.js host no-npm "test-80-*" — 3/3 (compression pipeline)

Consolidate CLI (string[]) and programmatic (PkgExecOptions) entry points
through a single canonical ParsedInput shape, dropping the
options->argv->parseArgs round trip. Split resolveConfig into named phase
helpers (resolveInput, resolveConfigFile, resolveOutput, resolveTargetList,
assignTargetOutputs). Fully resolve flags in the config layer: compress
becomes CompressType enum, targets becomes NodeTarget[] with per-target
output paths and input-overwrite guards applied. Expose effective merged
pkg on ResolvedConfig so index.ts no longer duplicates precedence logic.
Drop unused minimist dep; util.parseArgs drives CLI parsing.
@robertsLando robertsLando changed the title refactor(config): unify flag registry, phase-split resolveConfig feat(config): accept CLI-only build flags in pkg config Apr 23, 2026
@robertsLando robertsLando requested a review from Copilot April 23, 2026 12:53
Integration-only coverage (test-50-config-flags) exercises a handful of
end-to-end paths via real binaries and takes seconds. This test requires
lib-es5/config directly and runs 78 assertions in ~200ms covering:

- CLI parseInput: positionals, short-circuits, all flag kinds (bool/string/
  list), short aliases, alias collapsing (target/targets, out-path/outdir/
  out-dir), every FLAG_SPECS bool negation, positional/negation last-wins,
  unknown-option rejection
- Programmatic parseInput: type guards, input validation, bakeOptions/
  publicPackages/noDictionary/targets array-join, empty-array behavior
- resolveFlags: CLI>config>default merge for every flag, three-state
  override (CLI false beats config true), list empty-clear, whitespace
  trimming, csv parsing, '*' preservation, all compress aliases
- validatePkgConfig: unknown-key warn, known-key silence, type mismatches
  for bool/string/list, mixed-array rejection
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Extends pkg’s configuration system so that build-shaping flags previously only available via CLI can also be declared in pkg config files, with consistent precedence (CLI > config > defaults) and centralized flag/schema handling.

Changes:

  • Introduces a FLAG_SPECS registry and new lib/config.ts pipeline to parse CLI/programmatic input and resolve merged config + flags.
  • Refactors exec() to consume a single resolved config/flags shape, removing the previous minimist-based parsing path.
  • Adds documentation and a new test suite validating config-driven flags, CLI overrides, type errors, and unknown-key warnings.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
lib/config.ts New centralized parsing/validation/resolution for config + flags + targets/output.
lib/index.ts Refactored to use parseInput() + resolveConfig() output instead of minimist + ad-hoc resolution.
lib/types.ts Expands PkgOptions schema to include newly supported config keys (and makes dictionary optional).
lib/help.ts Adds help text noting config support for build-shaping flags.
docs-site/guide/configuration.md Documents newly accepted config keys in the schema table.
test/test-50-config-flags/* New tests and fixtures covering config-driven flags, overrides, warnings, and type errors.
package.json Removes minimist and @types/minimist.
yarn.lock Lockfile updated accordingly.

Comment thread lib/config.ts Outdated
Comment thread docs-site/guide/configuration.md Outdated
Comment thread docs-site/guide/configuration.md Outdated
Comment thread package.json
Comment thread lib/help.ts Outdated
Comment thread lib/types.ts Outdated
- Validate `pkg` config field is a plain object (reject string/array/null)
  with a clear error message instead of silently falling back to defaults
- types.ts: `PkgCompressType` now excludes numeric reverse-mapping keys
  from the enum (`Exclude<keyof typeof CompressType, number>`)
- help.ts: add `--no-signature` to the options section; list
  `native-build` and `sea` in the config-overridable flags note
- docs: correct `targets` type (`string | string[]`) and rename
  `deployAssets` row to `deployFiles` (matches actual code/types)
- test-80-compression fixture: swap `minimist` require for `picomatch`
  (direct dep — minimist was removed from package.json)
- test-50-config-flags: new `pkg.not-object.json` fixture + assertion
  for the non-object pkg rejection
@robertsLando robertsLando requested a review from Copilot April 23, 2026 13:14
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.

Comment thread lib/config.ts Outdated
Comment thread lib/help.ts Outdated
Comment thread lib/types.ts
Comment thread test/test-80-compression/test-x.js Outdated
robertsLando and others added 3 commits April 23, 2026 15:24
- fix "give" → "given" typo in --sea help text
- preserve last-wins order for --target/--targets aliases via token walk
- narrow PkgOptions.deployFiles to `[from, to]` / `[from, to, 'directory']` tuples
- force-require picomatch/picocolors in compression fixture for real VFS payload

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both --target and --targets are niche aliases; last-wins token walk
is overkill. Revert to simple nullish collapse.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.

Comment thread lib/index.ts
Comment on lines +154 to 156
if (flags.debug) {
log.debugMode = true;
}
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exec() only sets log.debugMode when flags.debug is true, but never resets it to false. If exec() is invoked multiple times in the same process (programmatic API), a first call with debug: true will leave debug logging enabled for subsequent calls even when debug is false/unspecified. Set log.debugMode unconditionally from the resolved flag value (e.g., assign log.debugMode = flags.debug) so each invocation is isolated.

Suggested change
if (flags.debug) {
log.debugMode = true;
}
log.debugMode = flags.debug;

Copilot uses AI. Check for mistakes.
Comment thread lib/help.ts
@@ -23,7 +23,12 @@ export default function help() {
--fallback-to-source if bytecode generation fails for a file, ship it as plain source instead of skipping it
--no-dict comma-separated list of packages names to ignore dictionaries. Use --no-dict * to disable all dictionaries
-C, --compress [default=None] compression algorithm = Brotli, GZip, or Zstd (Zstd requires Node.js >= 22.15)
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The help output documents --no-signature but not the positive --signature flag, even though the CLI parser supports both and --signature is needed to override a config value of signature: false. Add a corresponding --signature line (or otherwise document how to explicitly enable signing) to keep the CLI reference accurate.

Suggested change
-C, --compress [default=None] compression algorithm = Brotli, GZip, or Zstd (Zstd requires Node.js >= 22.15)
-C, --compress [default=None] compression algorithm = Brotli, GZip, or Zstd (Zstd requires Node.js >= 22.15)
--signature explicitly enable macOS binary signing

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(config): accept the CLI-only build flags in the config file

2 participants