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
2 changes: 1 addition & 1 deletion ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ _Second external review, this one of the codebase rather than the experience. Ra
### Architecture, once the gates are in

- [ ] **Staged resolution with diagnostics** — `normalizeConfig` now applies defaults, coerces conflicts, derives helpers, and encodes framework/target policy in one pass, and **silently** disables what it can't support. Split into ordered stages returning `{ config, changes[], warnings[], errors[] }` so "we turned Storybook off because this isn't a component library" is reported rather than inferred.
- [ ] **A resolved domain model** — the config carries ~16 derived booleans (`isReact`, `hasApp`, `publishable`, `usesVite`…) that can in principle contradict each other. Features should consume `resolved.targets` / `resolved.build` / `resolved.package` instead of re-interpreting raw selections.
- [x] ~~**A resolved domain model**~~ — **Shipped (3.1).** `normalizeConfig` now attaches a grouped `cfg.resolved` view (`targets` / `language` / `framework` / `build` / `module` / `package`) so consumers can read structured state (`cfg.resolved.targets.app`) instead of re-interpreting raw selections; the deployment contract already does. The reviewer's real worry — "flags can in principle contradict" — is now **proven impossible**: `test/invariants.test.js` asserts across a broad matrix that the grouped view never disagrees with the flat flags and that the derived state is internally consistent (publishable ⟺ its definition, an app is always ESM and never publishable, at most one framework, gated options off when their precondition fails, …). A future `normalizeConfig` change that introduces a contradiction fails there. Deliberately *not* done: a mechanical rewrite of every `cfg.hasApp` reference to `cfg.resolved.targets.app` — it's byte-safe but pure cosmetic churn; the flat flags stay supported and the invariants test guarantees they can't drift.
- [x] ~~**Make feature ordering explicit**~~ — **Shipped (3.1).** Rather than build speculative phase/requires/conflictsWith machinery, the real risk — order silently deciding a collision — is now an enforced invariant: `test/feature-registry.test.js` asserts every feature has a unique id and that **no two features collide** on a file or package.json field across the full config matrix (1683 configs). That immediately surfaced a real one — `bundler` and `service` both set `scripts.dev` on `node-service`, resolved only by array position — now fixed by making the service the explicit owner of its `dev` script. Ordering is no longer load-bearing for correctness; a colliding new feature fails the test instead of shipping.
- [ ] **Pairwise combination testing** — the integration matrix covers presets, which are the *common* paths. The risk is cross-feature interaction (dual + Rollup + Jest + Biome). Generate a matrix where every option pair appears at least once.
- [x] ~~**Dependency version catalog**~~ — **Shipped (3.1).** Every template version now lives in `src/core/versions.js` (`V` for specs, `PEER` for peer ranges, `HELD` for deliberately-held deps + reasons, `LAST_REVIEWED`). All 16 feature/monorepo modules reference it instead of hard-coding specs — verified **byte-identical across 3740 configs**, and a conformance test asserts every generated version comes from the catalog (it caught a wrong `ts-jest` spec and two missing deps while being built). The freshness workflow imports `HELD` instead of duplicating it. A bump is now one line: proven by fixing #19 (`release-it` ^20 → ^21) in the catalog alone.
Expand Down
11 changes: 11 additions & 0 deletions docs/packkit-core.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,19 @@ function normalizeConfig(input = {}, diagnostics = null) {
if (!(cfg.hasService || cfg.hasCli)) coerce("env", false, "ENV_REQUIRES_SERVICE_OR_CLI", "Env validation was disabled because it only applies to a service or CLI.");
if (cfg.release !== "changesets") coerce("canary", false, "CANARY_REQUIRES_CHANGESETS", "Canary releases were disabled because they require the Changesets release flow.");
if (!(cfg.isTs && cfg.hasLibrary && !cfg.hasFramework && !cfg.hasApp)) coerce("jsr", false, "JSR_REQUIRES_PLAIN_TS_LIBRARY", "JSR publishing was disabled because it applies only to a plain TypeScript library.");
cfg.resolved = resolvedView(cfg);
return cfg;
}
function resolvedView(cfg) {
return {
targets: { library: cfg.hasLibrary, cli: cfg.hasCli, service: cfg.hasService, app: cfg.hasApp },
language: { typescript: cfg.isTs, ext: cfg.ext, srcExt: cfg.srcExt },
framework: { name: cfg.framework, react: cfg.isReact, vue: cfg.isVue, svelte: cfg.isSvelte, any: cfg.hasFramework },
build: { vite: cfg.viteBuild, custom: cfg.customBuild, usesVite: cfg.usesVite, has: cfg.hasBuild },
module: { format: cfg.moduleFormat, esm: cfg.hasEsm, cjs: cfg.hasCjs },
package: { publishable: cfg.publishable, monorepo: cfg.monorepo }
};
}

// src/core/render.js
var UNSAFE_KEYS = /* @__PURE__ */ new Set(["__proto__", "constructor", "prototype"]);
Expand Down
23 changes: 23 additions & 0 deletions src/core/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -353,5 +353,28 @@ export function normalizeConfig(input = {}, diagnostics = null) {
if (cfg.release !== 'changesets') coerce('canary', false, 'CANARY_REQUIRES_CHANGESETS', 'Canary releases were disabled because they require the Changesets release flow.');
// JSR is TypeScript-first, ESM, and for plain (non-framework) libraries.
if (!(cfg.isTs && cfg.hasLibrary && !cfg.hasFramework && !cfg.hasApp)) coerce('jsr', false, 'JSR_REQUIRES_PLAIN_TS_LIBRARY', 'JSR publishing was disabled because it applies only to a plain TypeScript library.');

cfg.resolved = resolvedView(cfg);
return cfg;
}

/**
* A grouped, read-only view of the derived state — the same booleans the flat
* `cfg.has*`/`is*` flags expose, organized by concern. Consumers that prefer
* structured access (`cfg.resolved.targets.app`) can use it instead of
* re-interpreting raw selections; the flat flags remain for existing code.
*
* It is derived purely from the flags computed above, and test/invariants.test.js
* asserts the two never disagree and that the state is internally consistent —
* so the derived model can't silently contradict itself.
*/
function resolvedView(cfg) {
return {
targets: { library: cfg.hasLibrary, cli: cfg.hasCli, service: cfg.hasService, app: cfg.hasApp },
language: { typescript: cfg.isTs, ext: cfg.ext, srcExt: cfg.srcExt },
framework: { name: cfg.framework, react: cfg.isReact, vue: cfg.isVue, svelte: cfg.isSvelte, any: cfg.hasFramework },
build: { vite: cfg.viteBuild, custom: cfg.customBuild, usesVite: cfg.usesVite, has: cfg.hasBuild },
module: { format: cfg.moduleFormat, esm: cfg.hasEsm, cjs: cfg.hasCjs },
package: { publishable: cfg.publishable, monorepo: cfg.monorepo },
};
}
14 changes: 8 additions & 6 deletions src/embedded/contract.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,25 @@
export function deriveDeploymentContract(cfg) {
const run = (script) => (cfg.packageManager === 'npm' ? `npm run ${script}` : `${cfg.packageManager} ${script}`);
const start = cfg.packageManager === 'npm' ? 'npm start' : `${cfg.packageManager} start`;
// Read the structured resolved view rather than re-interpreting raw selections.
const { targets, build } = cfg.resolved;

if (cfg.hasService) {
if (targets.service) {
// The generated server reads PORT but defaults to 3000, so PORT is optional,
// not required — `port` communicates the default and the `PORT` env var
// overrides it. `healthCheckPath` is only asserted because every service
// framework Packkit generates (Hono/Fastify/Express) defines /health.
return prune({
type: 'node-service',
buildCommand: cfg.hasBuild ? run('build') : undefined,
buildCommand: build.has ? run('build') : undefined,
startCommand: start,
port: 3000,
healthCheckPath: '/health',
requiredEnvironmentVariables: [],
});
}

if (cfg.hasApp) {
if (targets.app) {
return prune({
type: 'static',
buildCommand: run('build'),
Expand All @@ -38,16 +40,16 @@ export function deriveDeploymentContract(cfg) {
});
}

if (cfg.hasCli) {
if (targets.cli) {
return prune({
type: 'cli',
buildCommand: cfg.hasBuild ? run('build') : undefined,
buildCommand: build.has ? run('build') : undefined,
});
}

return prune({
type: 'library',
buildCommand: cfg.hasBuild ? run('build') : undefined,
buildCommand: build.has ? run('build') : undefined,
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/embedded/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const MAX_DEFINITION_BYTES = 50_000_000;
const UNSAFE_KEYS = new Set(['__proto__', 'constructor', 'prototype']);

/** Non-OPTIONS config keys the pipeline sets itself; not "unknown". */
const KNOWN_EXTRA_KEYS = new Set(['preset', 'generatorVersion']);
const KNOWN_EXTRA_KEYS = new Set(['preset', 'generatorVersion', 'resolved']);

// Fields where a host override silently changing an existing value is worth a
// diagnostic (matches pkg-merge's protected set + dependency maps).
Expand Down
79 changes: 79 additions & 0 deletions test/invariants.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import { test } from 'node:test';
import assert from 'node:assert/strict';
import { normalizeConfig, fromPreset, PRESET_NAMES } from '../src/core/index.js';

// The config carries a lot of derived state — flat flags (isTs, hasApp,
// publishable…) and the grouped cfg.resolved view. These assert that the derived
// state is internally consistent and can never silently contradict itself, so a
// future change to normalizeConfig that introduces a contradiction fails here.

// A broad matrix of resolved configs.
function matrix() {
const out = [];
const langs = ['ts', 'js'];
const modules = ['esm', 'dual', 'cjs'];
const frameworks = ['none', 'react', 'vue', 'svelte'];
const targetSets = [['library'], ['cli'], ['service'], ['app'], ['library', 'cli']];
for (const preset of PRESET_NAMES) out.push(fromPreset(preset, { name: 'x' }));
for (const language of langs)
for (const moduleFormat of modules)
for (const framework of frameworks)
for (const target of targetSets) {
out.push(normalizeConfig({ name: 'x', language, moduleFormat, framework, target }));
out.push(normalizeConfig({ name: 'x', language, moduleFormat, framework, target, monorepo: true }));
}
return out;
}

test('the grouped resolved view never disagrees with the flat flags', () => {
for (const cfg of matrix()) {
const r = cfg.resolved;
assert.equal(r.targets.library, cfg.hasLibrary);
assert.equal(r.targets.cli, cfg.hasCli);
assert.equal(r.targets.service, cfg.hasService);
assert.equal(r.targets.app, cfg.hasApp);
assert.equal(r.language.typescript, cfg.isTs);
assert.equal(r.framework.any, cfg.hasFramework);
assert.equal(r.framework.react, cfg.isReact);
assert.equal(r.build.has, cfg.hasBuild);
assert.equal(r.module.esm, cfg.hasEsm);
assert.equal(r.package.publishable, cfg.publishable);
}
});

test('derived state is internally consistent — no contradictions possible', () => {
for (const cfg of matrix()) {
const label = `${cfg.language} ${cfg.moduleFormat} ${cfg.framework} [${cfg.target}]${cfg.monorepo ? ' monorepo' : ''}`;
const ok = (cond, msg) => assert.ok(cond, `${msg} — for ${label}`);

// A target is always present.
ok(cfg.target.length > 0, 'has at least one target');
// Publishable is exactly libraries/CLIs that are neither an app nor a service.
ok(cfg.publishable === ((cfg.hasLibrary || cfg.hasCli) && !cfg.hasApp && !cfg.hasService), 'publishable definition holds');
// An app is bundled, never published, and always ESM.
if (cfg.hasApp) {
ok(!cfg.publishable, 'an app is not publishable');
ok(cfg.moduleFormat === 'esm', 'an app is ESM-only');
}
// A service is never publishable.
if (cfg.hasService) ok(!cfg.publishable, 'a service is not publishable');
// Framework flags are mutually exclusive and agree with `any`.
const fw = [cfg.isReact, cfg.isVue, cfg.isSvelte].filter(Boolean).length;
ok(fw <= 1, 'at most one framework');
ok(cfg.hasFramework === (fw === 1), 'hasFramework matches exactly one concrete framework');
// Options that require a publishable package must be off when it isn't.
if (!cfg.publishable) {
ok(!cfg.pkgChecks, 'pkgChecks off when not publishable');
ok(!cfg.sizeLimit, 'sizeLimit off when not publishable');
ok(!cfg.sourcemaps, 'sourcemaps off when not publishable');
}
// Storybook only for a component library (framework, library, not app).
if (cfg.storybook) ok(cfg.hasFramework && cfg.hasLibrary && !cfg.hasApp, 'storybook only on a component library');
// E2E only for an app.
if (cfg.e2e) ok(cfg.hasApp, 'e2e only on an app');
// JSR only for a plain TS library.
if (cfg.jsr) ok(cfg.isTs && cfg.hasLibrary && !cfg.hasFramework && !cfg.hasApp, 'jsr only on a plain TS library');
// Canary only rides the changesets flow.
if (cfg.canary) ok(cfg.release === 'changesets', 'canary requires changesets');
}
});