fix(types): the DataSource contract carries deleteView's per-home outcomes (#4564) - #4569
Merged
Merged
Conversation
…comes (#4564) PR #4562 (#4479) widened the ObjectStack adapter's deleteView to return DeleteViewResult { deleted, draft?, published? }, but the shared interface still declared the narrow Promise of { deleted: boolean }. Nothing failed to compile — a wider return is assignable to a narrower declaration — so the adapter satisfied the interface while every consumer reaching it THROUGH DataSource was handed a type with the per-home outcomes already discarded. DeleteViewResult and ViewHomeDeleteOutcome move to packages/types/src/data.ts beside the interface that returns them, and deleteView?'s declared return widens to Promise of DeleteViewResult (optionality and parameters unchanged). data-objectstack imports them for its own use and re-exports both names, so every existing importer keeps compiling and now resolves to the same declaration the shared contract speaks. Census before the move: zero importers of either name outside the declaring file, PR #4562's own suite included. Pinned by deleteViewContract.types.test.ts. The pins are compile-time because the defect is: vitest erases types, so the consumer read below was green against the narrow declaration too — measured, 7/7 green with the fix reverted, while tsc returned 14 diagnostics. A live NarrowLegacyResult control keeps the discrimination honest. Note that `satisfies` and `extends` assertions are green in BOTH worlds for the same assignability reason the gap exploited; only the type IDENTITY assertions discriminate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
PM step-7 复核 — ACCEPT (session_017Qqyix2QcnpUC9XeYVDzx3)
Auto-merge armed (squash) — landing verified per the merge-queue discipline. Generated by Claude Code Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 13, 2026 11:00
This was referenced Aug 13, 2026
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 #4564
PR #4562 (#4479) widened the ObjectStack adapter's
deleteViewto returnDeleteViewResult { deleted, draft?, published? }. The shared interface did not follow:DataSource.deleteView?still declared the narrowPromise< { deleted: boolean } >.Nothing failed to compile, and that is precisely what made the gap invisible — a wider return is assignable to a narrower declaration, so the adapter satisfied the interface while every consumer reaching it through
DataSourcewas handed a type with the per-home outcomes already discarded. Today's single real call site (app-shell'sObjectViewdelete handler) awaits the call and reads nothing off the receipt, so the loss was latent rather than broken.Implements the PM ruling on the card (option a).
What changed
ViewHomeDeleteOutcomeandDeleteViewResultnow live inpackages/types/src/data.ts, beside theDataSourceinterface that returns them.DataSource.deleteView?'s declared return widens toPromise< DeleteViewResult >. Still optional, same two parameters — the growth is to the return type only, anddeletedis untouched.packages/data-objectstack/src/index.tsimports both names for its own use and re-exports them under the same names. Zero behavior lines:deleteView's implementation,readViewDeleteReceipt, andmetadata-client.tsare untouched.packages/types/src/index.tsadds both names to the./dataexport block — required for entry-reachability, and the basis of thetypesminor grading.The direction was forced. The dependency runs
@object-ui/data-objectstackto@object-ui/typesand never the other way, so the shapes could not be imported downward. Moving them was the alternative to re-declaring a structural twin intypes— which the one-resolver rule rejects, because a copy stays mutually assignable with the original for exactly as long as it takes to drift.Importer census, measured before the move
Repo-wide grep for both spellings on
origin/mainat537a0d19c: zero importers outside the declaring file itself — PR #4562's owndeleteView.homes.test.tsincluded, which types its adapter asanyand never names either type. The re-export is therefore insurance for future importers, not a load-bearing shim. The onlyDataSource.deleteViewcall site repo-wide ispackages/app-shell/src/views/ObjectView.tsx:1237-1238, which reaches it through(dataSource as any)and reads nothing; it is unedited and compiles unchanged.Red first
The new pins were written and run before the fix.
pnpm --filter @object-ui/data-objectstack type-checkon unmodifiedmainreturned 14 diagnostics, headlined by the issue's own sentence:plus seven
TS2344: Type 'false' does not satisfy the constraint 'true'on the identity assertions. All 14 are green after the change.Reverse verification. The three source files were taken back out with
git diff+git checkout --(nevergit stash),@object-ui/typeswas rebuilt so the check read the reverted declarations rather than a freshdist, and the same 14 diagnostics returned byte-identically. Restored viagit apply,sha256sum -cOK on all three.Two honest notes on where the pins get their colour
Both are stated in the test file's header rather than left for a later reader.
The pins are compile-time because the defect is. vitest transpiles with esbuild and erases types. Measured: with the fix reverted, the new file is 7/7 green under vitest while
tscreturns 14 errors. The colour comes fromtype-check, which this package'stsconfig.jsonapplies to its wholesrc/**, tests included.A
satisfies/extendsassertion does not discriminate here, and I did not pretend otherwise. The card asked for an explicit satisfies-style assertion, and it is present — but measured on unmodifiedmain, bothadapter.deleteView satisfies ContractDeleteViewand theextendsform are green in both worlds, for exactly the assignability reason the gap exploited. Only the type identity assertions (Equal< ... >) go red. The file carries both and says which one is load-bearing.A live
NarrowLegacyResultcontrol models the pre-#4564 declaration, so "the contract used to lose the per-home outcomes" stays an assertion the compiler re-checks rather than a comment.Must-not-change, verified
deleteView.homes.test.ts— untouched, all 15 pins green.viewCacheInvalidation.pin.test.ts— green, untouched.deleteView?still optional, same two parameters,deletedstillboolean— pinned.ObjectView.tsxandmetadata-client.ts— not edited; app-shell type-checks green.Emitted .d.ts, measured both ways
Clean
dist/andtsconfig.tsbuildinfobetween builds.@object-ui/types— additive: two new exported interfaces, the widened return, the entry re-export. minor (entry-reachable growth).@object-ui/data-objectstack— not byte-identical, so this is the ruling's "declaration moved" branch: the twointerfaceblocks leavedist/index.d.tsand become a re-export from@object-ui/types(121.61 KB to 120.25 KB). Both names remain in the public export list, so no importer breaks, but the declaration genuinely relocated and the emitted types now depend on@object-ui/typesfor it. minor, not patch.Neither is major, per the repository's version-alignment convention (the major tracks
@objectstack, never an API-break count).Verification
All heavy steps serialized on the shared
flockwith a capped heap.pnpm --filter "@object-ui/types" --filter "@object-ui/data-objectstack" type-check— both Done.pnpm exec vitest run --maxWorkers=2 packages/data-objectstack/ packages/types/— 66 files, 895 tests passed (baseline before the change: 65 / 888).pnpm --filter "...@object-ui/types" type-check— the consumer direction, not the dependency one — 42 of 47 workspace projects, 0 errors. Full workspace build (43/43) run first, because the sweep initially hit a genuine TS2307 stale-artifact error in@object-ui/collaboration.no-unused-varswarnings on its type aliases; that is the repo's existing type-pin idiom, which already emits 29 of them inqueryDataset.test.tsonmain.check:control-bytesOK (4315 tracked files),check:phantom-depsOK,changeset:checkOK. Self-scan withgrep -naPover every touched file: clean.Generated by Claude Code