Repository navigation
Conversation
Deleting an item invalidates its entity but leaves its id in each Collection that held it. Pushing (or unshifting) an item with the same id then appended a second copy of the id, so the list showed the item twice while All() counted it once. push and unshift now drop an id already in the list before adding it, so the item moves to the end or start. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
🦋 Changeset detectedLatest commit: d140e03 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The Query now reads the same list the cart page renders, and only its new Query line is highlighted. Remove-then-add counts correctly with #4487. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05d723f54f
ℹ️ 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".
|
Size Change: +186 B (+0.17%) Total Size: 112 kB 📦 View Changed
ℹ️ View Unchanged
|
|
Docs preview: https://docs-site-pr-4487-data-client.vercel.app Built from d140e03 merged into |
There was a problem hiding this comment.
Benchmark React
Details
| Benchmark suite | Current: d140e03 | Previous: 8817f47 | Ratio |
|---|---|---|---|
data-client: getlist-100 |
146.18 ops/s (± 7.7%) |
222.22 ops/s (± 8.5%) |
1.52 |
data-client: getlist-500 |
51.28 ops/s (± 4.9%) |
73.27 ops/s (± 6.4%) |
1.43 |
data-client: update-entity |
312.5 ops/s (± 7.2%) |
666.67 ops/s (± 6.1%) |
2.13 |
data-client: update-user |
333.33 ops/s (± 7.8%) |
625 ops/s (± 6.8%) |
1.88 |
data-client: getlist-500-sorted |
55.87 ops/s (± 8.6%) |
64.78 ops/s (± 9.3%) |
1.16 |
data-client: update-entity-sorted |
277.78 ops/s (± 7.4%) |
444.66 ops/s (± 10.7%) |
1.60 |
data-client: update-entity-multi-view |
312.5 ops/s (± 6.2%) |
555.56 ops/s (± 7.0%) |
1.78 |
data-client: list-detail-switch-10 |
10.27 ops/s (± 7.6%) |
20.64 ops/s (± 8.0%) |
2.01 |
data-client: update-user-10000 |
81.3 ops/s (± 8.6%) |
125.79 ops/s (± 11.2%) |
1.55 |
data-client: invalidate-and-resolve |
40.98 ops/s (± 5.2%) |
60.61 ops/s (± 7.1%) |
1.48 |
data-client: unshift-item |
250 ops/s (± 1.5%) |
384.62 ops/s (± 0.0%) |
1.54 |
data-client: delete-item |
222.22 ops/s (± 12.6%) |
555.56 ops/s (± 9.1%) |
2.50 |
data-client: move-item |
166.67 ops/s (± 6.0%) |
307.77 ops/s (± 6.0%) |
1.85 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4487 +/- ##
==========================================
+ Coverage 98.37% 98.41% +0.04%
==========================================
Files 161 161
Lines 3080 3101 +21
Branches 605 612 +7
==========================================
+ Hits 3030 3052 +22
Misses 10 10
+ Partials 40 39 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: d140e03 | Previous: 7adefee | Ratio |
|---|---|---|---|
normalizeLong |
435 ops/sec (±3.85%) |
693 ops/sec (±3.56%) |
1.59 |
normalizeLong Values |
397 ops/sec (±1.06%) |
607 ops/sec (±1.11%) |
1.53 |
normalizeLong Scalar |
370 ops/sec (±3.66%) |
544 ops/sec (±2.39%) |
1.47 |
normalizeLong Scalar update |
914 ops/sec (±0.80%) |
1574 ops/sec (±1.86%) |
1.72 |
denormalizeLong |
246 ops/sec (±6.26%) |
349 ops/sec (±5.61%) |
1.42 |
denormalizeLong Values |
239 ops/sec (±4.60%) |
322 ops/sec (±3.99%) |
1.35 |
denormalizeLong donotcache |
985 ops/sec (±1.06%) |
1820 ops/sec (±2.03%) |
1.85 |
denormalizeLong Values donotcache |
745 ops/sec (±0.12%) |
1151 ops/sec (±5.18%) |
1.54 |
denormalizeLong Scalar donotcache |
1052 ops/sec (±0.28%) |
1909 ops/sec (±0.48%) |
1.81 |
denormalizeShort donotcache 500x |
1418 ops/sec (±0.14%) |
2462 ops/sec (±1.91%) |
1.74 |
denormalizeShort 500x |
678 ops/sec (±6.93%) |
899 ops/sec (±7.30%) |
1.33 |
denormalizeShort 500x withCache |
6961 ops/sec (±0.08%) |
9181 ops/sec (±2.20%) |
1.32 |
queryShort 500x withCache |
3262 ops/sec (±0.07%) |
5359 ops/sec (±0.74%) |
1.64 |
buildQueryKey All |
59263 ops/sec (±0.94%) |
98489 ops/sec (±0.41%) |
1.66 |
query All withCache |
6212 ops/sec (±2.62%) |
10256 ops/sec (±5.77%) |
1.65 |
denormalizeLong with mixin Entity |
225 ops/sec (±7.29%) |
335 ops/sec (±6.11%) |
1.49 |
denormalizeLong withCache |
6914 ops/sec (±0.20%) |
12871 ops/sec (±0.43%) |
1.86 |
denormalizeLong withCache (Scalar churn) |
6851 ops/sec (±1.01%) |
12670 ops/sec (±1.00%) |
1.85 |
denormalizeLong Values withCache |
5125 ops/sec (±1.60%) |
9804 ops/sec (±1.03%) |
1.91 |
denormalizeLong Scalar withCache |
7890 ops/sec (±0.22%) |
13747 ops/sec (±0.99%) |
1.74 |
denormalizeLong Scalar update withCache |
4156 ops/sec (±0.42%) |
7523 ops/sec (±0.38%) |
1.81 |
denormalizeLong All withCache |
6370 ops/sec (±0.29%) |
13111 ops/sec (±0.23%) |
2.06 |
denormalizeLong Query-sorted withCache |
6498 ops/sec (±2.51%) |
10436 ops/sec (±6.87%) |
1.61 |
denormalizeLongAndShort withEntityCacheOnly |
1770 ops/sec (±0.22%) |
3035 ops/sec (±1.13%) |
1.71 |
denormalize bidirectional 50 |
4752 ops/sec (±10.75%) |
7229 ops/sec (±9.22%) |
1.52 |
denormalize bidirectional 50 donotcache |
41461 ops/sec (±0.55%) |
74415 ops/sec (±0.62%) |
1.79 |
getResponse |
4478 ops/sec (±4.14%) |
6850 ops/sec (±6.61%) |
1.53 |
getResponse (null) |
10023771 ops/sec (±0.98%) |
19028450 ops/sec (±3.52%) |
1.90 |
getResponse (clear cache) |
215 ops/sec (±7.50%) |
333 ops/sec (±7.84%) |
1.55 |
getSmallResponse |
3568 ops/sec (±0.94%) |
6392 ops/sec (±0.50%) |
1.79 |
getSmallInferredResponse |
2857 ops/sec (±0.09%) |
4838 ops/sec (±0.54%) |
1.69 |
getResponse Collection |
4663 ops/sec (±2.00%) |
8037 ops/sec (±2.20%) |
1.72 |
get Collection |
3427 ops/sec (±0.18%) |
6524 ops/sec (±0.49%) |
1.90 |
get Query-sorted |
4972 ops/sec (±1.75%) |
7759 ops/sec (±5.02%) |
1.56 |
setLong |
437 ops/sec (±1.31%) |
737 ops/sec (±0.35%) |
1.69 |
setLongWithMerge |
247 ops/sec (±0.71%) |
415 ops/sec (±1.86%) |
1.68 |
setLongWithSimpleMerge |
264 ops/sec (±0.13%) |
443 ops/sec (±1.35%) |
1.68 |
setSmallResponse 500x |
935 ops/sec (±0.77%) |
1611 ops/sec (±1.41%) |
1.72 |
setMany 50x one-per-row |
160 ops/sec (±0.34%) |
220 ops/sec (±1.51%) |
1.38 |
setMany 50 batch |
3702 ops/sec (±0.44%) |
6279 ops/sec (±0.55%) |
1.70 |
setMany 500x one-per-row |
16.37 ops/sec (±0.51%) |
22.67 ops/sec (±0.44%) |
1.38 |
setMany 500 batch |
1441 ops/sec (±0.18%) |
2821 ops/sec (±0.35%) |
1.96 |
This comment was automatically generated by workflow using github-action-benchmark.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6add6a52c6
ℹ️ 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".
push and unshift append like Array.push() again. Before adding, a create drops refs to deleted (INVALID) entities from the matching collections, so an item re-created with a deleted pk shows once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
Checks each incoming item's pk before normalizing it, so a create into a large collection no longer scans every member when nothing was deleted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22c9d55c02
ℹ️ 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".
…leted refs Drop the deleted refs inside the merge, so the stored list keeps its meta and out-of-order responses still reorder. Ids passed as refs aren't processed, since normalize passes them through. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
Record which incoming items were deleted, then drop the refs normalize produced for them, so the ref format lives in one place. Hoist the test meta fixture and drop a test the Union deletion test already covers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
State that ArraySchema.normalize keeps one ref per input item, and test a push where a deleted item follows a live one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Build the repeated todos lists and their meta with one helper, and drop a call-site comment the helper's docblock already covers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Requested by Nathaniel · project thread
Motivation
Bug fix. A list is keyed by another entity's id, for example cart lines keyed by product, or followed users. The app deletes an item, then creates an item with the same id and adds it through
pushorunshift.Instance: in the computed-properties cart demo (#4484), the reader removes the mug's last unit (
CartResource.delete), then adds the mug again (getList.push). The cart list shows the mug twice. AQueryovergetList.schemacounts 5 items instead of 4. Tracked as Linear REACT-13.Solution
delete(anInvalidateschema) marks the entity INVALID and leaves its id in every Collection that held it. Denormalize skips INVALID entities, so the stale id stays hidden until the same pk is created again. Then both the stale id and the pushed one show, so from the user's sidepushadded two items.push,unshift,addWith) normalizes its input, it checks each incoming item's pk. If that entity is currently deleted, the create drops that id from the matching collections as part of the merge, then adds as usual. This check has to happen before normalizing, because normalizing revives the entity.pushandunshiftkeepArray.push()/unshift()semantics: nothing is deduped or moved, and pushing an id the list already shows (not deleted) still adds it again.merge/mergeWithStoresee the list without the dropped ids), so the list's meta andshouldReorderbehave as before.process/pk/getEntityper incoming item. Only when an incoming item was deleted does the merge filter the list. Ids passed as refs are skipped, since normalize passes them through without processing.Assumptions and validation
High: the new tests fail on master and pass here. They cover a deleted then re-created id for
push,unshiftand a Union collection holdingnull, and a collection the create doesn't target staying untouched. A live duplicate is still added (Array semantics), list meta is kept for an out-of-order create, and ids passed as refs aren't processed. All package suites pass locally (1937 tests), andtscis clean.High, V8 check (
.cursor/rules/v8-optimization.mdc, Haiku,examples/benchmarkspread suite, "collection push 10 onto 10k", 8 interleaved runs per ref):key/schema/process/isSingleSchemaAn earlier version that scanned every member of each collection measured -7.9%, which is why only the incoming items are checked now. The benchmark has no deleted entities, so the drop path (
withoutRefs, added after this run) isn't measured. It runs only when an incoming item was deleted, and it doesn't change the no-deletion path.Medium:
isShallowEqualgains anullguard, becauseremoveMergenow also runs against Union lists that holdnull.Not covered: Immutable.js state that stores a collection as a
Listkeeps the old behavior (the drop only applies to arrays).Generated by Claude Code
🤖 Generated with Claude Code
https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
Note
Medium Risk
Touches core Collection normalization/merge for all push/unshift creates; behavior change is scoped to deleted-entity re-adds but affects shared cache state for nested lists.
Overview
Fixes a duplicate list entry when an entity is invalidated (deleted) and later re-added via Collection
pushorunshift(e.g.getList.push). Invalidate leaves the old id in the stored collection while denormalize hides it; re-creating the same pk then merged both the stale ref and the new one.normalizeCreatenow scans incoming items before normalize (so entities are not revived yet), finds pks whose store entry isINVALID, and merges through awithoutRefswrapper that strips those refs from the existing list before the usual push/unshift merge.isShallowEqualgains null checks so removal works on Union lists that containnull. NormalArray.push/unshiftsemantics are unchanged: live duplicates are still appended, and only targeted collections are updated.Adds regression tests (including Union members) and a patch changeset; the v0.19 blog notes the fix.
Reviewed by Cursor Bugbot for commit d140e03. Bugbot is set up for automated code reviews on this repo. Configure here.