fix(build): let turbo own build order, delete the hand-listed prebuild hooks - #7986
Merged
Merged
Conversation
…d hooks `packages/components` hand-listed the workspace packages its build needed (`prebuild`: types, core, react; `pretest`: the same chain) and `apps/site` did the same for the schema catalog it already declares as a dependency. A hand-written chain is a second, unchecked copy of the package's own `dependencies`, and the components one had drifted: `@object-ui/react` imports `@object-ui/i18n` and `@object-ui/data-objectstack`, neither of which the list names, so on a tree where those two have no `dist` the chain dies inside react's `tsc` with TS2307 -- pointing at react's source rather than at the stale list. turbo derives the same closure from the dependency graph and gets seven packages. Both hooks are deleted. turbo's `build.dependsOn: ["^build"]` (and `test.dependsOn: ["^build"]`) is now the single source of build order, which is what every path CI takes already used. Usage change: the bare per-package forms no longer build upstream first. `turbo run build --filter=<pkg>` / `turbo run test --filter=<pkg>` and the repo-root `pnpm test` are the supported entry points; every in-repo document and hint string that taught the direct form now teaches the turbo form, and the root `site:build` script goes through turbo the way `ci.yml` already did. The `turbo build inputs` guard's delegation assertion is ratcheted to zero rather than deleted: it used to police the survivors (each delegated package must be a declared dependency), which is the weaker guard -- a delegation whose target IS declared still duplicates the graph and still drifts. The derivation keeps collecting delegations so that re-introducing one fails loudly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013uAaxiwgYDybsTNV9xwa1M
This was referenced Sep 6, 2026
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Sep 6, 2026
Contributor
Author
|
Standing down on
Proceeding: verification round ACCEPTed on #7292 ( Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7292
Triage picked option 1. The two hand-written
pre*hooks are deleted and turbo's dependency graph is now the single source of build order.What changed
packages/components/package.jsonprebuild(types, core, react) andpretest(which pointed at it)apps/site/package.jsonprebuild(the schema catalog)package.jsonsite:buildnowturbo run build --filter=@object-ui/site, matching whatci.ymlalready ranpackages/components/README_SHADCN_SYNC.mdscripts/build-plugin-stylesheet.mjs,scripts/shadcn-sync.js,packages/components/src/__tests__/page-header-action-ids.dist.spec.tsxscripts/__tests__/turbo-build-inputs.test.ts,scripts/__tests__/helpers/build-program.ts.changeset/7292-turbo-owns-build-order.mdThe defect, and the reading behind the fix
The card's reproduction was run by its author on one tree, before and after, with a live control. I did not re-run it -- see "What is NOT verified" -- so it is quoted, not re-measured:
The hand-written chain names three packages; the closure turbo derives is seven (
core,data-objectstack,i18n,react,react-runtime,sdui-parser,types). A hand-written chain is a second, unchecked copy of the package's owndependencies, and this one had drifted.turbo.jsonreading (measured on this branch's base,3702f92a9):Both
buildandtestdepend on^build, so turbo orders the upstream builds for both tasks from the declared dependency graph. That is what makes the hooks redundant rather than load-bearing -- and it is also why deletingpretestdoes not breakturbo run test.Usage change (the one thing reviewers should read twice)
pretestis what letpnpm --filter @object-ui/components testbuild its upstream packages first. After this PR the bare per-package forms no longer do that:turbo run build --filter=@object-ui/components,turbo run test --filter=@object-ui/components, and the repo-rootpnpm testpnpm --filter @object-ui/components build,pnpm --filter @object-ui/components testHonest statement of the residual state (the card's A4): after this deletion
pnpm --filter @object-ui/components buildon a clean tree still fails -- now on this package's own upstreamdistrather than inside@object-ui/react. That is not a regression this PR introduces; the direct form was never a supported entry point on a cold tree, and the deleted hook was a partial, drifting imitation of the ordering turbo already does correctly. The remedy strings and docs now say so.Every in-repo site that taught the direct form was found with a wider grep than the one on the card (the card's narrower sweep over
AGENTS.md/README.md/content/docs//.github/legitimately returned zero -- none of the hits live there):apps/site: the dependency edge exists, so the hook goesJudged by the same rule the card sets for it. The measurement is the manifest itself:
The edge is present, so turbo's
build.dependsOn: ["^build"]already orders@object-ui/example-schema-catalogbefore@object-ui/site. This is the "delete it" case, not the "the edge is missing, fix the manifest" case -- no dependency was added, andturbo.jsonis untouched.One consequence had to be handled rather than assumed:
ci.yml:1545builds the site aspnpm turbo run build --filter='@object-ui/site'(turbo form, unaffected), but the rootsite:buildscript used the direct form and would have silently stopped building the catalog. It is rewritten to the turbo form.Why the delegation guard is ratcheted, not deleted
scripts/__tests__/turbo-build-inputs.test.tscarried a liveness pin -- "no package delegates a build any more" -- whose failure message instructs deleting the assertion along with the narrowing it defends. Deleting both hooks trips exactly that pin.I did not delete it. The predecessor policed the survivors (each delegated package must be a declared dependency), which is the weaker guard: a delegation whose target IS declared still duplicates the graph, still drifts, and still fails only on the trees nobody looks at --
apps/sitewas precisely that shape, declared target and all. The assertion is inverted into a ratchet on the whole class ("no build lifecycle script delegates to another package withpnpm --filter"), andhelpers/build-program.tskeeps collecting delegations unchanged, so re-introducing one fails loudly here instead of silently widening an unswept build program. Under the decision frame this is the "contract tightening beats consumer leniency / declared = enforced" axis, and it adds no new gate, no new file and no new verification surface -- it replaces one assertion in a test that already ran.Verified (R45) -- every leg the R44 report left NOT RUN has now been run
Verification round on this exact head (
16aed838e), no code change needed: the branch is byte-identical to what R44 pushed, andgit diff HEADis empty after every probe. Dedicated worktree,pnpm install --frozen-lockfileEXIT=0. "Cold tree" below means measured: 0packages/*/dist+apps/*/dist, 0tsconfig.tsbuildinfo, no turbo cache directory. Heavy runs went through this container's shared verify lock (itsVERDICT command-exitline is the exit code, not a bare status variable); every exit code was captured by redirecting to a file first, never through a pipe.pnpm exec vitest run scripts/__tests__/turbo-build-inputs.test.tsTest Files 1 passed (1)/Tests 46 passed (46); the verbose reporter names the rewritten assertion as passing:no build lifecycle script delegates to another package with pnpm --filterprebuildtopackages/components/package.json, re-run leg 1Testsreads 1 failed, 45 passed, 46 total, received["@object-ui/components -> @object-ui/core, @object-ui/react, @object-ui/types"]against expected[]. Mutation proven on disk (git hash-objectmoved 12055578 to e6a070ba, HEAD blob 12055578); restore proven (git checkout HEAD -- path, hash back to 12055578,git diff HEAD0 lines)turbo run build --filter=@object-ui/components --concurrency=2Tasks: 8 successful, 8 total/Cached: 0 cached, 8 total. Zero cache hits, so this is an executed ordering, not a replay: the seven upstream packages plus this oneturbo run build --filter=@object-ui/site --concurrency=2Tasks: 30 successful, 30 total/Cached: 8 cached, 30 total. turbo reportsRemote caching disabled, using shared worktree cache, and the 8 hits are leg 3a's own outputs in this same tree, so leg 3c re-ran it forced--force, on a re-wiped cold treeTasks: 30 successful, 30 total/Cached: 0 cached, 30 total, with@object-ui/example-schema-catalog:build: cache bypass, force executingat line 1032 and@object-ui/site:buildat line 1037. This is the ordering claim with no cache in itpnpm site:buildTasks: 30 successful, 30 total/Cached: 30 cached, 30 total(warm tree). A3 holds: the turbo form still puts the catalog ahead of the siteturbo run build --filter=@object-ui/site --dry=json, before and after deleting theworkspace:*edge fromapps/site/package.json@object-ui/example-schema-catalog#buildpresent at index 3 against site at index 28, and listed verbatim in@object-ui/site#build's owndependencies. After deleting the edge: 30 tasks, catalog absent from both the task list and site's dependencies. So the manifest edge is what orders it, never the deleted hook. Restore proven (hash back to 161ab80b,git diff HEAD0 lines)pnpm --filter @object-ui/components build, cold treeERR_PNPM_RECURSIVE_RUN_FIRST_FAIL @object-ui/components@17.6.0 build. The PR body's residual-state claim is confirmed and sharpened: the failure has MOVED. Before the change it died inside@object-ui/react'stsc; now it dies in this package's ownvite build, on its own upstream dist -- 253 TS2307 lines naming@object-ui/core(100),@object-ui/types(83),@object-ui/react(36),@object-ui/i18n(25),@object-ui/types/form(4),@object-ui/sdui-parser(3),@object-ui/react-runtime(2). Not a regression this PR introduces; the direct form was never self-sufficient on a cold treepnpm test:distTasks: 9 successful, 9 total;Test Files 1 passed (1)/Tests 3 passed (3)forpage-header-action-ids.dist.spec.tsx. Run throughtest:distand not as a bare vitest path: the.dist.spec.tsxsuffix is collected ONLY by thedistproject, so a bare path run would have matched zero tests and read as greenscriptssuitepnpm exec vitest run scripts/__tests__/Test Files 107 passed (107)/Tests 3240 passed (3240). Owed because the diff edits two files in it;helpers/build-program.tshas exactly one reader,turbo-build-inputs.test.tsgit grep -nfor the two hook names,prebuildandpretest, excludingCHANGELOG.mdandpnpm-lock.yamlhelpers/build-program.tsthat still COLLECTprebuild-- which is precisely what makes leg 2 go redGates, each quoting its own verdict line:
Two counts moved since the card was written, both benign and both explained rather than waved through:
Tasks: 9 successful, 9 totalfor the components build; on this base it is 8. The closure is unchanged at seven upstream packages plus the package itself; the card's run was on an older base.@object-ui/test-support#build, whosecommandin the dry JSON is the literal string NONEXISTENT -- the package declares nobuildscript, so turbo lists it in the graph and executes nothing.Open questions from the R44 report, decided on the measurement: question 1 -- A stands. The ratchet is green on this branch (leg 1) and red, naming the offending package, the moment a hook comes back (leg 2). It is neither weakened nor deleted. Question 2 -- A stands, the extra files are kept; leg 3d is the measurement that says the root
site:buildrewrite was forced rather than opportunistic.Session attribution as prose, because the footer block does not survive an edit: created by
https://claude.ai/code/session_013uAaxiwgYDybsTNV9xwa1M, verified byhttps://claude.ai/code/session_01MM7kaS4dPpYHV5BsMyu4tQ.Census re-count on this base
Re-counted on
3702f92a9: 42packages/*/package.json+apps/*/package.json. Hand-writtenpre*build/test hooks: exactly the two this PR deletes. (packages/runner,apps/consoleand the two console examples declarepreview/prepublishOnly, andapps/sitedeclarespostinstall: fumadocs-mdx-- none of those is a build-order copy, and none is touched.)Out-of-scope finding, not fixed here
scripts/setup.sh:55-70is the same defect class outside the manifest census: it hand-liststypes -> core -> react -> components -> fields -> layoutand, underset -e, itspnpm --filter @object-ui/react buildstep fails on a clean checkout for the identical TS2307 reason, before this PR and after it. This PR does not change that file's behaviour (it already built types/core/react itself, so the deletedprebuildwas redundant there). Separately, its last step callspnpm test:root, which the rootpackage.jsondoes not declare. Both left untouched and reported for triage; #7292 is the only card this PR closes.R45 update: both findings are now filed, unassigned, as objectui#7987 -- the hand-listed six-package chain and the undeclared
pnpm test:rootcall. Re-measured on this branch and unchanged by it: underset -ethe script dies at itspnpm --filter @object-ui/react buildstep, three lines before it ever reachespnpm --filter @object-ui/components build, so the deleted hook was never what carried it.Generated by Claude Code