feat(bindx): add selection-erased entity view types - #83
Closed
matej21 wants to merge 1 commit into
Closed
Conversation
Consumers hand a selection-branded accessor to a helper typed for a
different selection and bridge the gap with `as unknown as`. The
diagnosis this was meant to fix - "the __selected brand is invariant" -
is wrong. `readonly __selected?: TSelected` is a readonly optional
property, i.e. an output position, and is already covariant: widening a
full accessor to a narrower selection compiles today.
What actually fails is the opposite direction, for three reasons, and
only one of them is the brand:
1. narrow -> full is rejected by the brand AND, independently, by
EntityFieldsRef/EntityFieldsAccessor, which are keyed on
`keyof TSelected` and so are missing the properties outright. It
SHOULD be rejected: EntityHandle.fields throws UnfetchedFieldError
for any field outside the selection, so widening in place is a real
runtime bug. Loosening the brand would legalise it.
2. A free `TSelected` in a generic helper cannot be resolved at all - no
variance annotation can fix an unknowable mapped-type key set.
3. TEntityName is invariant through FieldRefMeta.entityType, so a
`string`-named accessor does not flow into a literal-named parameter.
A second, independent cast generator.
So instead of changing an existing type, add two erased views:
EntityRefLike<TEntity> = EntityRefInterface<TEntity, unknown>
EntityAccessorLike<TEntity> = EntityRefLike<TEntity> & { $data }
They erase TSelected and TEntityName, keep __entityType as the
discriminator, and deliberately omit the field proxy. Use them in
parameter positions that need entity identity and the
selection-independent API. A receiver that reads fields must still
declare the selection it needs - that cast was hiding a bug.
Nothing existing is modified. `__selected` also turned out to be
load-bearing for inference, not just checking: replacing it with
`unknown` breaks selection inference across ~30 sites.
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.
Consumer applications repeatedly bridge accessor types with
as unknown as. This started as "make the__selectedbrand covariant so those casts go away".That diagnosis is wrong
__selectedis declared three times inhandles/types.ts(HasManyRef:182,HasOneRefInterface:239,EntityRefInterface:317) as:A readonly optional property is an output position — it is already covariant. Widening a full accessor to a narrower selection compiles today; verified in all three intersection members.
TBrandis unused in every body, and__schema?: TSchema & anycollapses toany.What actually fails is the other direction, for three distinct reasons, only one of which is the brand:
1. narrow → full is rejected twice over, and should be. By the brand, and independently by
EntityFieldsRef/EntityFieldsAccessor, which are keyed onkeyof TSelectedand are therefore literally missing the properties. Loosening the brand cannot fix this — and must not:Widening a narrow selection in place is a real runtime bug. The compiler is right to reject it, and some of the casts in consumer code are suppressing a crash, not a type-system artifact.
2. A free type parameter is unfixable by variance in principle.
EntityFieldsRef<Website, Website>→EntityFieldsRef<Website, TSelected>withTSelectedunresolved cannot be accepted by any variance annotation, because the mapped type's key set is unknowable.3.
TEntityNameis a second, independent cast generator (not in the original report). It is invariant throughFieldRefMeta<TEntityName>.entityType+__entityName, soEntityAccessor<W,W,B,string>does not flow intoEntityAccessor<W,W,B,'Website'>. Hooks produce literal names;createComponent'sBuildEntityPropsproducesstring.Making the brand bivariant was considered and rejected: it is both a hole (it legalises the
UnfetchedFieldErrordirection) and insufficient (the mapped types still reject).__selectedalso turned out to be load-bearing for inference, not just checking — replacing it withunknownbroke selection inference across ~30 sites. Anything that touches it ripples widely.What this adds instead
Two additive types. No existing type is modified.
They erase
TSelectedandTEntityName, keep__entityTypeas the discriminator, and deliberately omit the field proxy. Use them in parameter positions that need entity identity plus the selection-independent API (id,$isNew,$persistedId,$errors,$on,$intercept, …).EntityAccessorLikealso admitsHasOneAccessor— a has-one structurally satisfiesEntityRefInterface. Intentional, and asserted.What this does and does not buy
EntityAccessor<Website>keeps failing. The zero-opt-in alternative — redefiningEntityRef<T>'s defaultTSelectedfromTEntitytounknown— would strip field access fromEntityRef<T>repo-wide; not attempted.$state === 'connected'narrowing case.HasOneAccessoris a plain intersection, not a discriminated union; no guard can narrow$entity. That needsHasOneAccessorrestructured into a union keyed on$state— a separate, larger change.as unknown astouching ref/accessor types was checked; each has a different cause (class→structural-type impedance, collector proxies, role narrowing). The repeater'snewEntity as unknown as EntityAccessor<TEntity>is a genuine instance, but its fix is to threadTSelectedintoRepeaterPreprocessCallback, becausepreprocessneeds field access.Soundness
15 tests in
tests/unit/types/selectionErasure.test.ts. Still rejected, all asserted: unrelated entities both directions; pointer-only ref (no$data);HasManyRefandFieldRef; plain objects including{ id: string; $data: unknown }; erasure is one-way (EntityRefLike<W>↛EntityRef<W>); no field proxy ('title' | 'slug' | '$fields'∉keyof EntityRefLike<Website>,'id'∈); the unsound widening stays illegal; and the brand is not vacuous.Negative assertions use
assertFalse<IsAssignable<S, T>>()withIsAssignable<S,T> = [S] extends [T] ? true : false(tuple-wrapped so unions do not distribute), matching the existing idiom intests/typeSafety.test.ts. This is deliberately not@ts-expect-error, which is banned here and is in any case satisfied by any error on the line, including an unrelated typo.Evidence the negatives bite, beyond "it compiles": weakening
EntityRefLiketoPartial<…>makes 5 negative assertions fail to compile.One documented limit: with a free type parameter the conditional stays deferred (
boolean), so "full accessor ↛EntityRef<W, TSelected>" cannot be asserted false. There is a comment in the file rather than a faked assertion.Gates
bun run typecheckexit 0 ·bun test --path-ignore-patterns='**/tests/browser/**'1752 pass / 0 fail / 150 files (baseline 1737 / 149).Follow-up
Not re-exported from
@contember/bindx-react. If a consumer only depends on the React package, these two names need adding to its root export — one line, left out of this PR's scope.