linksToMany per-slot JS-access contract: Card | undefined#5025
Merged
habdelra merged 6 commits intoMay 30, 2026
Conversation
… undefined The plural linksToMany getter returned a WatchedArray whose broken slots held raw link sentinels, forcing every consumer to recognize and skip them. Make the per-slot read surface uniform: arr[i] is now Card | undefined, never a sentinel. - watched-array.ts: optional hideSlot predicate; a numeric-index read whose backing value matches resolves to undefined. length, iteration count, and in-place swaps are unchanged. A rawValues escape hatch hands the backing array to the link-aware base modules. - card-api.gts: LinksToMany creates its WatchedArray with hideSlot isNonPresentLink; the not-loaded scan, serialize, queryableValue, and the lazilyLoadLink swap loops read the raw backing array so they still see and swap sentinels. - field-support.ts: getRelationship reads the raw backing array, remaining the typed surface that reports each slot's true state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d301a2aea4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Per-slot index access is masked (Card | undefined), so rebuilding a linksToMany field from masked reads dropped every *other* broken slot's sentinel, losing the broken reference + error doc (it serialized as data: null). Rebuild from the raw backing array instead: - add (card + file): append to rawArrayValues(field) rather than spreading the masked value. - remove: filter rawArrayValues(field) by position. - reorder: hand ember-sortable the raw per-slot entry as its opaque item model (the masked value is undefined for broken slots — non-unique and lossy across a reorder); setItems then assigns the reordered raw entries. The editor never inspects the sentinels — it only preserves them by position. Tests: removing a present sibling keeps the broken element's placeholder + URL; typing into a sibling field with a broken element present keeps input focus (the broken slot does not destabilize the edit form). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens the JavaScript surface contract for linksToMany arrays so that per-slot index access (arr[i]) returns Card | undefined and never exposes link sentinel objects (not-loaded / link-error / link-not-found) to consumers, while still keeping sentinels in the backing store to preserve lazy-load swaps, serialization, and relationship introspection.
Changes:
- Add a
hideSlotoption toWatchedArray’s proxygettrap to mask sentinel slots asundefined, plus arawValues/rawArrayValues()escape hatch for internal link-aware logic. - Update
LinksToManylogic to scan/serialize/swap using the raw backing array so sentinel behavior remains correct while userland reads are masked. - Update
getRelationship()to read the raw backing array so relationship state reporting remains accurate, and add integration tests covering the new contract.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/host/tests/integration/components/linksto-many-per-slot-access-test.gts | Adds integration coverage for per-slot Card | undefined behavior across present, not-loaded, and terminal broken link slots. |
| packages/base/watched-array.ts | Adds hideSlot masking for numeric indices and a rawValues/rawArrayValues() escape hatch to access the sentinel-bearing backing array. |
| packages/base/field-support.ts | Makes getRelationship() operate on the raw backing array so it can still report true per-slot relationship states. |
| packages/base/card-api.gts | Ensures linksToMany lazy-load scanning, serialization, queryable values, and swap loops operate on the raw backing array while preserving in-place swap semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…rray masking - serialize iterates the raw backing array with forEach (it only populates the relationships map; the mapped array was discarded). - watched-array get trap consults hideSlot only for in-range indices, so an out-of-range read falls through to undefined without handing the predicate a missing slot. - top-of-file comment describes the generic T | undefined masked surface (Card | undefined for the linksToMany caller), not Card-specifically. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The field value is typed `any`, so `rawArrayValues(...)` inferred `unknown[]` and the `(_c: CardDef, ...)` filter predicate failed the overload check. Pin the type parameter and let the predicate params infer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Preview deploymentsHost Test Results 1 files 1 suites 1h 48m 20s ⏱️ Results for commit d336ec0. Realm Server Test Results 1 files ±0 1 suites ±0 11m 1s ⏱️ + 1m 10s Results for commit d336ec0. ± Comparison against earlier commit da0c7d8. |
…ersal Three CI failures, all rooted in the new Card | undefined per-slot contract: - Two existing producer/serialization tests waited on the broken state by probing the bucket WatchedArray per-slot (`arr.some(isLinkNotFound)`, `e?.type === 'not-loaded'`). Per-slot reads are now masked, so those probes never saw the sentinel and the waitUntil timed out. Settle on the typed `getRelationship` surface instead. - A prerender test card's computed linksToMany did `this.friends[0].friends` with no guard. During the lazy-load window the first slot reads as `undefined` (it would resolve on a later render), so the unguarded property access threw. This is correct contract behavior — card authors must defensively guard computed traversals for missing links — so optional-chain the traversal. Verified: gc-card-store's reachability walk already guards array elements with `item && typeof item === 'object'`, so masked undefined slots are safely skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…kstomany-per-slot-js-access-contract
backspace
approved these changes
May 29, 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.
Background and Goal
A plural
linksToManyfield whose elements failed to load held link sentinels(
not-loaded/link-error/link-not-found) directly inside itsWatchedArray. Readingarr[i]handed those sentinel objects to userland, soevery consumer had to recognize and skip them to render a list.
This pins down the per-slot accessor contract:
arr[i]is nowCard | undefined— never a sentinel.
arr[i] === <the card>arr[i] === undefined, and the read triggers its lazy loadarr[i] === undefined, terminal (no retrigger)arr.lengthstill counts every slot, including broken onesmapsurfaceundefinedfor broken slots, sofor (const c of arr) if (c == null) continue;skips them andarr.map(c => c?.name)yieldsundefinedper broken slotstill re-renders
Where to start
packages/base/watched-array.ts— theProxynow takes an optionalhideSlotpredicate; a numeric-index read whose backing value matches resolves to
undefined. The sentinel stays in the backing array (solength, iterationcount, and in-place swaps are unaffected). A
rawValuessymbol /rawArrayValueshelper hands the backing array to the link-aware base modules.
packages/base/card-api.gts—LinksToManybuilds itsWatchedArraywithhideSlot: isNonPresentLink; the not-loaded scan,serialize,queryableValue, and thelazilyLoadLinkswap loops read the raw backingarray so they still observe and swap sentinels.
packages/base/field-support.ts—getRelationshipreads the raw backingarray, remaining the typed surface that reports each slot's true state (the
render path's broken-link placeholder reads through it, not through
arr[i]).Key decisions
read concern; the sentinels still drive lazy-load swaps, serialization, and
getRelationship, so they must remain stored. Only per-slot index access ismasked.
hideSlotis injected, not imported.WatchedArraystays generic and link-agnostic;
card-apisuppliesisNonPresentLink, avoiding awatched-array → field-support → card-apiimport cycle.containsManyarrayspass no predicate and are unaffected.
production code outside
card-api.gts,field-support.ts, andwatched-array.tsobserves a sentinel through per-slot access.Closes CS-11220.