Skip to content

✨ feat(nx): automate the workspace with Nx - #226

Open
roninjin10 wants to merge 10 commits into
mainfrom
build-compare/nx
Open

✨ feat(nx): automate the workspace with Nx#226
roninjin10 wants to merge 10 commits into
mainfrom
build-compare/nx

Conversation

@roninjin10

@roninjin10 roninjin10 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

What this is

A complete, modern Nx 23.1.1 (nx@latest) setup for the flows workspace, built to be compared side by side with the Turborepo and Bazel sibling PRs and with the in-repo BUILD.ts system. Nothing existing was deleted or rewritten: the pnpm scripts, BUILD.ts, ci.yml, and release.yml are untouched, and the Nx lane runs next to them. The full analysis is in docs/build-systems/nx.md — it is the real deliverable.

What was wired

  • Inference-first configuration (nx.json, no hand-written project.json except the crate): @nx/vitest is registered against every vitest.config.ts (under Nx 23's merge order the package.json script wins the test name collision, so the script stays the command — the plugin contributes config-aware hashing; the doc details this), @nx/vite is registered for Vite builds, @nx/js/typescript is registered with inference disabled (why below). Package check/lint/test/build/circular/coverage scripts become cacheable targets through targetDefaults. One project needed a project.json: crates/flows-jj, because it is not an npm package — it models cargo-fmt, cargo-clippy, cargo-test, and the wasm reproducibility gate with explicit inputs.
  • namedInputs that model this repo: sharedGlobals covers the root configs that invalidate everything (tsconfig.base.json, eslint.jsdoc.js, eslint.boundaries.js, pnpm-workspace.yaml, rust-toolchain.toml); production excludes the test/lint surface. check/test use ["default", "^default"] — not ^production — because cross-package imports resolve to source here (exports: { ".": "./src/index.ts" }), so dependency sources must be in the cache key. Verified live: editing pnpm-workspace.yaml re-hashed and re-ran all 51 test tasks.
  • Module boundaries from smthrs.group (eslint.boundaries.js + a two-line edit in each of the 45 package eslint configs): the local plugin tools/nx/plugin.ts maps the manifest field onto group:engine|agent|tooling tags, and @nx/enforce-module-boundaries encodes the real constraint — engine (the published train) depends on engine only; agent on engine+agent; tooling on everything; packages never on apps. The current graph is clean (66 engine→engine edges, zero engine→agent/tooling), and the rule is verified firing on synthetic violations.
  • nx release (nx.json#release): fixed release group over tag:group:engine, conventional-commits versioning, updateDependents: auto for internal range retargeting, workspace changelog, v{version} tags. release.yml stays; this is the alternative path. Dry-run output below.
  • Caching + affected in CI (.github/workflows/nx.yml): nrwl/nx-set-shas@v4, nx affected -t check,lint,fmt,circular,test,build --parallel=2, actions/cache over .nx/cache. Remote caching: Nx Cloud is the current recommended path and needs an account we do not have. The workflow is fully wired for it — the one human step is pnpm exec nx connect (writes nxCloudId) plus adding the NX_CLOUD_ACCESS_TOKEN secret. Until then the local cache carries CI. The OSS self-hosted options (community S3/GCS plugins) are unofficial; Powerpack is gone as a separate product in Nx 23.
  • A local plugin (tools/nx): tags from smthrs.group, plus a cacheable fmt target (dprint check) inferred from dprint.json — the half of the repo's lint gate the eslint plugin can't express.
  • A new-package generator (nx g ./tools/nx:new-package): the Nx equivalent of the NewPackage rule. It scaffolds a conventional package with no project.json and reads the manifest shape (script set, exports, publishConfig, pinned devDependencies) from an existing sibling so the scaffold can't drift. Verified: the generated package passed check, lint, fmt, test, build immediately with zero Nx-specific files.
  • Also: .nxignore (keeps the vendor/jj submodule out of the file index), nx graph, nx watch (no config needed), nx migrate as the upgrade path.

Measured numbers (this worktree, Apple Silicon)

Surface Cold Warm
check+fmt+circular — 123 tasks, 47 projects 1m42s 1.1s (123/123 cache hits)
build — 44 projects 43.7s–1m16s 219ms (44/44)
test — 51 projects, --parallel=2 2m48s 209ms (51/51)
nx affected after a one-line packages/crypto/src/index.ts change 31 of 52 projects, 61 check+test tasks, 2m47s
nx affected after a one-line packages/triggers (leaf) change 1 project, 6 tasks, 6.2s

nx release --dry-run completes end to end: resolves 0.1.0 from disk (no git tags exist yet), derives a minor bump from conventional commits, plans the fixed engine train at 0.1.1 with internal ranges retargeted, renders the workspace changelog, and skips publish. nx graph --file renders the 52-project graph. The crate targets run (cargo-fmt verified locally).

What does not work, precisely

  • @nx/eslint inference is not registered. Its inferred command is hard-coded to eslint .; these flat configs deliberately cover only src, and eslint . fails on every package (parse errors on eslint.config.js and scripts/*.mjs, which no files block configures). The command can't be overridden without per-project files, so lint targets stay script-based. The Nx-specific lint value (boundaries) is wired directly instead.
  • Test atomization is rejected. The @nx/vitest atomizer runs one target per test file; packages with coverage.enabled: true enforce suite-level thresholds, so atomized runs fail their own coverage gate. The plugin also bakes the config's reportsDirectory into outputs at graph time — these configs use join(tmpdir(), …-${process.pid}), producing escaping outputs like {workspaceRoot}/../../../../tmp/flows-core-coverage-54087 with a dead PID.
  • nx sync / @nx/js/typescript doesn't fit this workspace. The repo brief said project references are in use; they are not — zero references in any tsconfig, by design (source-first resolution). The sync generator's model is composite solution-style TS: nx sync:check reports all 51 projects as missing root references while nx sync writes "references": [] (its composite filter drops everything), leaving the gate permanently red. Wiring it for real means converting 45 packages to composite projects — a change to build semantics, not Nx config. The plugin is registered with inference disabled and the doc explains why.
  • @nx/vite/@nx/vitest are scoped to packages/**. They boot configs through Node ESM at graph time; the apps use bundler-style extensionless imports (apps/ui/vite.config.tsapps/shared/src/NativeAgent.ts imports ./Cards with no extension), which crashes graph construction. Apps keep script targets.
  • The wasm gate runs but can't be proven here. wasm-repro is modeled with cache: false on purpose — the gate proves a from-source rebuild reproduces the committed bytes on this host, and a remote cache hit from another host would mask exactly the nondeterminism it catches. It only runs on x86_64-unknown-linux-gnu (the script refuses other hosts), so on this Mac it is verified by configuration inspection, not execution.
  • No toolchain model, no secrets model, no install target. Cache keys don't include the Node/TypeScript versions; secrets are plain env vars; node_modules is ambient. These are the three places BUILD.ts is strictly more expressive, and the doc details each.

Pre-existing issues this branch surfaced (not caused by it)

At the original branch point, two gates were red on origin/main itself, verified byte-identical on a pristine worktree: dprint check failed on 14 packages' READMEs, and pnpm exec smthrs was not found after a fresh install (the package rename had mangled the build-cli bin name to "smithers build" while the docs-parity CI step still calls smthrs). Both were fixed on main after the branch point (39bd390d restores the bin, ceb784b6 reformats the READMEs). This branch is rebased onto those fixes and re-verified: pnpm exec smthrs docs '//...' runs green, dprint check passes on the 14 formerly failing packages, and the first post-rebase nx run-many -t fmt re-ran exactly the 14 packages whose READMEs changed and hit the cache for the other 31 — the inputs model tracking reality.

Still true and still worth knowing:

  • @smthrs/kernel@smthrs/platform-browser is a real runtime dependency cycle (each lists the other in dependencies). pnpm tolerates it; Nx's ^build task graph refused to schedule it. Builds here don't consume dependency artifacts, so the ordering edge is dropped and the cycle is allowlisted in the boundary rule with a comment.

Verdict versus BUILD.ts

Nx wins on change-impact analysis (nx affected is real and correct here), cache ergonomics, the graph as a visible product, boundaries-as-lint, and not having to operate a cache Worker. Our system wins on the typed toolchain as a value, generated root files as drift-checked graph outputs, declared secrets, node_modules as a target, and type-checked target construction — with Nx, a wrong inputs entry is a silent stale hit that nobody catches. The doc's migration section recommends adopting the nx.yml lane in advisory mode (the same pattern as the repo's own smthrs-shadow lane) and explicitly recommends against adopting solution-style references, test atomization, or eslint . inference unless the underlying repo conventions change first.

🤖 Generated with Claude Code

smithers-lane-agent added 10 commits August 18, 2026 14:36
namedInputs model this repo: sharedGlobals covers the root configs that
invalidate everything, production excludes the test and lint surface.
targetDefaults make the existing package scripts cacheable with correct
inputs; release selects the engine group as a fixed release train.
The plugin maps smthrs.group onto group:* project tags and infers a
cacheable fmt target from dprint.json. The generator is the Nx equivalent
of the NewPackage rule: it scaffolds a conventional package with no
project.json, reading the manifest shape from an existing sibling.
engine may depend on engine only; agent on engine and agent; tooling on
everything; packages never on apps. The known kernel/platform-browser
runtime cycle and the dev-time BUILD.ts cycles through @smthrs/targets
are allowlisted so the gate catches new violations.
…s targets

wasm-repro is deliberately cache: false: the gate proves a from-source
rebuild reproduces the committed bytes on this host, and a remote cache
hit from another host would mask the nondeterminism it catches.
Wired for Nx Cloud (NX_CLOUD_ACCESS_TOKEN) but functional without it via
actions/cache over .nx/cache; connecting the cloud workspace is one
documented command for a human.
packages/chain's tests dynamically import @smthrs/memory as a test
technique; the boundary rule reads any dynamic import as a lazy-loading
boundary and forbids the static src imports. Exempt the specifier, and
record the finding in the comparison doc.
The branch point predated 39bd390 (restores the smthrs bin the rename
mangled) and ceb784b (reformats the READMEs dprint was failing). The
adoption section claimed both failures reproduce on a pristine
origin/main; they no longer do. Record the rebase and the
post-rebase re-verification: the docs-parity gate is green, dprint
passes on the 14 formerly failing packages, and the first post-rebase
fmt run invalidated exactly the 14 packages whose READMEs changed.
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.

1 participant