diff --git a/ROADMAP.md b/ROADMAP.md index edca31d..87ddd47 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -107,7 +107,7 @@ _Second external review, this one of the codebase rather than the experience. Ra - [ ] **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. -- [ ] **Make feature ordering explicit** — features merge in array order, so order silently decides who wins. Give them ids, phases, `requires`, `conflictsWith`, and detect collisions rather than relying on position. +- [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. - [ ] **Dependency version catalog** — template versions are embedded across feature modules. Centralize them with compatibility notes and a last-reviewed date; the freshness workflow already treats them as product data, this makes that explicit. - [ ] **Unify monorepo and single-package generation** — `generate()` returns early for monorepos, and 2.9 added a second early return inside that. Modelling a project as workspaces (a plain package being a project with one) would stop features having to be implemented twice. diff --git a/docs/packkit-core.js b/docs/packkit-core.js index f52ed4b..4641a96 100644 --- a/docs/packkit-core.js +++ b/docs/packkit-core.js @@ -641,6 +641,7 @@ var bundler_default = { pkg.scripts.build = "tsc"; pkg.scripts.dev = "tsc --watch"; } + if (cfg.hasService) delete pkg.scripts.dev; if (build) pkg.scripts.prepublishOnly = pkg.scripts.build; pkg.scripts.clean = "rimraf dist"; if (build) pkg.devDependencies = { ...pkg.devDependencies, rimraf: "^6.0.0" }; diff --git a/src/core/features/bundler.js b/src/core/features/bundler.js index ff7f044..9b99976 100644 --- a/src/core/features/bundler.js +++ b/src/core/features/bundler.js @@ -73,6 +73,11 @@ export default { pkg.scripts.dev = 'tsc --watch'; } + // A service owns its `dev` script (tsx/node --watch on the server entry); the + // bundler only builds it. Don't also contribute a `dev` — that was a silent + // conflict the two features "resolved" purely by array order. + if (cfg.hasService) delete pkg.scripts.dev; + if (build) pkg.scripts.prepublishOnly = pkg.scripts.build; pkg.scripts.clean = 'rimraf dist'; if (build) pkg.devDependencies = { ...pkg.devDependencies, rimraf: '^6.0.0' }; diff --git a/src/core/features/index.js b/src/core/features/index.js index 1156093..a541833 100644 --- a/src/core/features/index.js +++ b/src/core/features/index.js @@ -1,5 +1,12 @@ // Feature registry. Order matters only for how package.json fragments merge // (later features deep-merge over earlier ones); files are keyed by path. +// +// Ordering contract: no two features may own the same file path or the same +// package.json field for a given config. That invariant is enforced by +// test/feature-registry.test.js across the whole config matrix, so ordering is +// never load-bearing for correctness — if a new feature collides with an +// existing one, the test fails rather than the winner being decided by position +// here. Each feature declares a unique `id`, surfaced in collision diagnostics. import meta from './meta.js'; import bundler from './bundler.js'; diff --git a/test/feature-registry.test.js b/test/feature-registry.test.js new file mode 100644 index 0000000..467cef6 --- /dev/null +++ b/test/feature-registry.test.js @@ -0,0 +1,60 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import features from '../src/core/features/index.js'; +import { createProject } from '../src/embedded/index.js'; +import { PRESET_NAMES } from '../src/core/index.js'; + +// Features merge their files and package.json fragments in array order, so a +// collision between two of them is "resolved" silently by position. These tests +// make that safe: every feature is uniquely identified, and no two features are +// allowed to fight over the same output across the whole config matrix — a new +// feature that collides fails here instead of shipping an order-dependent bug. + +test('every feature has a unique id', () => { + const ids = features.map((f) => f.id); + assert.ok( + ids.every((id) => typeof id === 'string' && id.length), + 'each feature declares a string id', + ); + assert.equal(new Set(ids).size, ids.length, `duplicate feature ids: ${ids.join(', ')}`); +}); + +test('no two features collide on a file or package.json field, across the matrix', () => { + const toggles = [ + {}, + { storybook: true }, + { knip: true }, + { jsr: true }, + { pkgChecks: true }, + { sizeLimit: true }, + { e2e: true }, + { env: true }, + { canary: true }, + { doctor: true }, + { minify: true }, + ]; + const linters = ['eslint-prettier', 'biome', 'oxlint']; + const hooks = ['simple-git-hooks', 'husky', 'lefthook']; + + const collisions = new Set(); + for (const preset of PRESET_NAMES) { + for (const t of toggles) { + for (const lint of linters) { + for (const gitHooks of hooks) { + let project; + try { + project = createProject({ preset, name: 'x', overrides: { ...t, lint, gitHooks } }); + } catch { + continue; // an invalid combination — validation covers those elsewhere + } + for (const d of project.diagnostics) { + if (d.code === 'FEATURE_FILE_COLLISION' || d.code === 'PACKAGE_FIELD_CONFLICT') { + collisions.add(`${d.code} on ${d.field} — ${d.message}`); + } + } + } + } + } + } + assert.deepEqual([...collisions], [], 'features collide (ownership is order-dependent)'); +});