Skip to content

fix: correct the install order, the husky feature and failure reporting - #14

Merged
gabitoesmiapodo merged 6 commits into
mainfrom
refactor/codebase-sweep
Sep 10, 2026
Merged

gabitoesmiapodo merged 6 commits into
mainfrom
refactor/codebase-sweep

Conversation

@gabitoesmiapodo

Copy link
Copy Markdown
Collaborator

Summary

No related issue.

Cleanup edited the scaffolded project's package.json after the package manager had already written the lockfile, so a fresh project failed npm ci. Fixing the order turned up more problems, so this is a full sweep: correctness first, then duplication, then a dependency audit.

Changes

  • Generated projects install cleanly, because cleanup now runs before the install and the lockfile matches package.json
  • Choosing the EVM husky feature keeps the git hooks, instead of deleting them and the dependencies behind them
  • A failed interactive install exits non-zero and removes the half-written project instead of reporting success
  • --info publishes the modes each stack accepts, and the validator reads that same list
  • default mode resolves feature requirements, which only custom did before
  • Unchecking one feature no longer unchecks another that happens to share its label
  • Post-install links say where they lead, rather than "here"
  • pnpm audit is clean; one of the advisories reached everyone who installed the package
  • Feature names are a type, so a renamed or mistyped feature fails to compile instead of being silently skipped

Acceptance criteria

  • A scaffolded project passes npm ci / pnpm install --frozen-lockfile with no further work
  • Every advertised feature does what its label says, on both stacks
  • A failed install leaves nothing behind and reports a non-zero exit code
  • The five repo checks pass, and the docs match the code

Test plan

Automated tests

  1. Run pnpm install
  2. Run pnpm lint, pnpm typecheck, pnpm test, pnpm build and pnpm knip
  3. Expect 203 passing tests and no errors from any of the five
  4. Run pnpm audit and expect no known vulnerabilities

Manual verification

  1. Check out this branch, then run pnpm install && pnpm build
  2. In an empty directory, run node <repo>/dist/cli.js --evm --ni --name check_evm --mode custom --features demo,subgraph,typedoc
  3. Enter check_evm and run pnpm install --frozen-lockfile; expect it to succeed, and vocs and .husky to be gone
  4. Back in the empty directory, run node <repo>/dist/cli.js --canton --ni --name check_canton --mode default
  5. Enter check_canton and run npm ci --dry-run; expect it to succeed, and git log to show the baseline commit

Breaking changes

EVM full mode now keeps the husky hooks, since full means every feature and husky is one. Delete .husky in the generated project to get the old result.

Checklist

  • Self-reviewed my own diff
  • Tests added or updated
  • Docs updated (if applicable)
  • No unrelated changes bundled in

Screenshots

None.

Cleanup edited the generated project's package.json after the package
manager had already written the lockfile, so the two disagreed and
`npm ci` failed in a fresh Canton project. Cleanup now rewrites the
lockfile from the patched manifest, and only when it changed something.

- EVM: the `husky` feature is real again. It was advertised in `--info`,
  in `--help` and in the wizard, but cleanup deleted the hook files and
  the dependencies whether you asked for it or not. Selecting it now
  keeps the hooks; deselecting it removes them as before.
- Feature removal is data-driven for both stacks. Features declare their
  `paths`, `scripts` and `dependencies` in `config.ts`, so the file lists
  live in one place instead of being repeated in `cleanupFiles.ts`.
- package.json is read once, guarded against a template that ships none,
  and written only when a value changed. Two read-modify-write passes
  became one, and an untouched manifest is no longer reformatted.
- Only removed directories strip scripts now. A script that merely
  mentions a removed file, such as `markdownlint CLAUDE.md`, survives.
A failed interactive install printed its error but exited 0 and left the
half-written project on disk, so a wrapper script could not tell success
from failure. The three operation steps now call `abortInstall`, which
removes the partial directory and sets the exit code.

- `--info` gained a `modes` list per stack. It used to describe `default`
  as valid for EVM while the validator rejected it, which broke the
  documented discovery flow for agents.
- `default` mode now resolves feature requirements the same way `custom`
  does, so a feature that depends on another can never be kept without it.
- The multiselect used `&&` where the inverse of an "and" needs `||`, so
  unchecking one item would also uncheck any item sharing its label.
- `cli.tsx` reads the stack flags through a returned result instead of
  inspecting the global exit code twice.
- The interactive steps take the resolved `mode` and `features` from
  `app.tsx` instead of each recomputing them. The review screen now lists
  what actually gets installed.
The code carried a mix of loose line comments and none of them showed up
in editor tooltips. Each one that was worth keeping is now jsdoc on the
declaration it describes; the rest said what the code already says and
are gone.

No behavior change.
`FeatureName` was an alias for `string`, so every feature name written as
a string literal went unchecked. Renaming `subgraph` in `config.ts` left
`isFeatureSelected('subgraph', ...)` compiling happily while quietly
reporting the feature as deselected.

It is now the union of the keys in `stackDefinitions`, so a rename turns
every stale reference into a compile error. Two side effects:

- `isFeatureNameValid` became a type guard, so validated `--features`
  input narrows from `string` to a feature name.
- `getFeatureEntries(stack)` replaces four `Object.entries` calls over the
  feature map, each of which needed its own cast.

The union caught a real slip in the tests, which asked for the
non-existent canton features `counter` and `e2e`.
Cleanup ran after the install, so it had to repair the lockfile the
package manager had just written. Running it first means the manifest is
already pruned when the install starts: one dependency resolution instead
of two, nothing installed only to be deleted, and a lockfile that matches
by construction. The lockfile repair pass is gone.

- One way to remove a dependency. Features declare `packages`, which the
  package manager uninstalls. The second field that hand-edited
  package.json existed only because the install always ran the template's
  `postinstall` script; that call now happens only when the script exists,
  so Canton can use the same field as EVM.
- Cleanup is one code path for both stacks. The paths a stack always
  removes, where it stages replacement files, and whether it commits the
  scaffold are `StackConfig` fields, not `stack === 'canton'` branches.
  The baseline commit moved to its own operation, which runs last.
- The three operation steps were the same component three times, down to
  a byte-identical progress block. They now share `StepProgress`, which
  owns the progress list, the error display and the failure path, so a new
  step cannot forget to report a failure.
- Feature names are typed inside the config too, so a mistyped `requires`
  or `ifFeature` fails to compile instead of being skipped at runtime.
- Validation reads the modes a stack accepts from the config, which is the
  same list `--info` publishes.
- One place prints the JSON failure envelope, and the caller recognises an
  already-reported error by type instead of by reading the exit code.
`pnpm audit` reported 14 advisories. One of them reached people who
install the package: ink pulls in ws 8.18.2, and the fix landed in
8.20.1. The other three are test tooling only.

- Added pnpm overrides for ws, vite, postcss and nanoid. Each is a patch
  or minor bump inside the major its parent already asks for, so nothing
  changes but the fix. `pnpm audit` is clean.
- The post-install screen linked with "here" and "the docs". Those labels
  say nothing on their own, and a terminal that cannot render a hyperlink
  shows only the label. They now name what they point at.
- The readme screenshot had no alt text and no height, so it was invisible
  to a screen reader and shifted the page while loading.
@gabitoesmiapodo gabitoesmiapodo self-assigned this Sep 9, 2026
@gabitoesmiapodo gabitoesmiapodo moved this from Backlog to In review in Canton - dAppBooster (#390) Sep 9, 2026
@gabitoesmiapodo
gabitoesmiapodo merged commit a6ed048 into main Sep 10, 2026
8 checks passed
@gabitoesmiapodo
gabitoesmiapodo deleted the refactor/codebase-sweep branch September 10, 2026 14:55
@github-project-automation github-project-automation Bot moved this from In review to Done in Canton - dAppBooster (#390) Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant