Skip to content

build/CI: no prettier --check gate; formatting drift accumulating on main #627

@bpowers

Description

@bpowers

Problem

Neither the pre-commit hook (scripts/pre-commit) nor CI runs a prettier formatting check for TypeScript/JavaScript. As a result, files can be committed prettier-dirty, and drift has already accumulated on main.

The pre-commit hook only checks Rust formatting (the [rust] Checking formatting... step). For TypeScript it runs eslint, but eslint uses eslint-config-prettier solely to disable rules that conflict with prettier (eslint.config.shared.js:68-69, comment: "Disable rules that conflict with prettier") -- it does not enforce prettier formatting. A grep of .github/workflows/ for prettier/format --check/needs-format returns nothing.

Evidence

Two independent observations during Phase 2 of the @simlin/engine wasm backend work confirmed this:

  1. Newly-created Phase 2 files were prettier-dirty yet passed the pre-commit hook.

  2. A whole-tree check run from src/engine currently flags 6 files that are not touched by the engine-wasm-sim branch and are byte-identical to main (verified again while filing):

    $ cd src/engine && npx prettier --check 'src/**/*.ts' 'tests/**/*.ts'
    [warn] src/internal/project.ts
    [warn] src/project.ts
    [warn] src/worker-backend.ts
    [warn] src/worker-protocol.ts
    [warn] src/worker-server.ts
    [warn] tests/direct-backend.test.ts
    [warn] Code style issues found in 6 files. Run Prettier with --write to fix.
    

These 6 are pre-existing drift on main, not introduced by the current work; they were intentionally left untouched to avoid mixing unrelated reformatting into the wasm-backend feature's review. There is likely similar drift in other packages (core, diagram, app, server, serve-web) -- the 6 above are just what surfaced from the engine-package check.

Why it matters

  • Developer experience / review hygiene: prettier-dirty files merge silently, then unrelated PRs get noisy reformatting diffs (or reviewers flag formatting that the tooling should have caught).
  • The repo already intends prettier to be the formatter (pnpm js-format / pnpm format exist and write changes), but nothing verifies it on commit or in CI -- so the intent isn't enforced.

Components affected

  • scripts/pre-commit
  • .github/workflows/ (CI)
  • All TypeScript packages: engine, core, diagram, app, server, serve-web

Suggested resolution

Two parts, ordering matters -- clean up first, then add the gate, or the gate's first run will be noisy:

  1. One-time prettier --write sweep to clear existing drift (the 6 engine files above, plus a whole-repo pnpm js-format pass). Do this as its own commit so the formatting-only churn is isolated.

  2. Add a prettier --check gate to the pre-commit hook and CI. A pnpm js-needs-format script already exists in the root package.json (find src -name '*.ts' -o -name '*.tsx' | egrep -v '/(lib(\.(browser|module))?|core)/' | xargs prettier -l) and is currently wired into nothing -- it's the natural gate to reuse, mirroring how pnpm rust-needs-format (cargo fmt -- --check) is already enforced by the Rust formatting step.

    Caveat to resolve when wiring it: the existing js-needs-format script excludes core/ directories via its egrep -v filter, so a faithful reuse would skip those. Decide whether to broaden the glob (drop the core exclusion, which appears aimed at generated lib*/ output rather than src/core) or accept the gap, and document the choice.

Discovered

Identified during Phase 2 of the @simlin/engine wasm simulation backend work (engine-wasm-sim branch). Tracking only -- not fixed as part of that work to keep the feature's diff scoped.

This is distinct from tech-debt.md item #60 ("No CI feature matrix"), which is about a Cargo feature matrix for Rust builds, not TypeScript prettier formatting.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions