feat: add Lazy schema for deferred relationship denormalization#3829
Open
feat: add Lazy schema for deferred relationship denormalization#3829
Conversation
Introduces schema.Lazy(innerSchema) that: - normalize: delegates to inner schema (entities stored normally) - denormalize: no-op (returns raw PKs unchanged) - .query getter: returns LazyQuery for use with useQuery() LazyQuery resolves entities lazily: - queryKey: delegates to inner schema if it has queryKey, otherwise passes through args[0] - denormalize: delegates to inner schema via unvisit (full entity resolution) No changes needed to EntityMixin or unvisit - Lazy.denormalize as no-op means the existing denormalize loop works without any special handling. Co-authored-by: natmaster <natmaster@gmail.com>
Tests cover: - Normalization: inner entities stored correctly through Lazy wrapper - Denormalization: Lazy field leaves raw PKs unchanged (no-op) - LazyQuery (.query): resolves array of IDs, delegates to Entity.queryKey, handles missing entities, returns empty for empty IDs - Memoization isolation: parent denorm stable when lazy entity changes - Stack safety: 1500-node bidirectional graph does not overflow Co-authored-by: natmaster <natmaster@gmail.com>
Documents the Lazy schema class including: - Constructor and usage patterns (array, entity, collection) - .query accessor for useQuery integration - How normalization/denormalization works - Performance characteristics Co-authored-by: natmaster <natmaster@gmail.com>
🦋 Changeset detectedLatest commit: d6ce627 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
|
Size Change: 0 B Total Size: 80.5 kB ℹ️ View Unchanged
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Benchmark React
Details
| Benchmark suite | Current: d6ce627 | Previous: 869f28f | Ratio |
|---|---|---|---|
data-client: getlist-100 |
172.41 ops/s (± 4.6%) |
175.44 ops/s (± 3.9%) |
1.02 |
data-client: getlist-500 |
47.39 ops/s (± 5.9%) |
42.19 ops/s (± 6.0%) |
0.89 |
data-client: update-entity |
434.78 ops/s (± 5.7%) |
416.67 ops/s (± 4.6%) |
0.96 |
data-client: update-user |
425.72 ops/s (± 8.4%) |
434.78 ops/s (± 0.0%) |
1.02 |
data-client: getlist-500-sorted |
52.49 ops/s (± 6.5%) |
50.76 ops/s (± 4.4%) |
0.97 |
data-client: update-entity-sorted |
400 ops/s (± 5.0%) |
370.37 ops/s (± 5.3%) |
0.93 |
data-client: update-entity-multi-view |
370.37 ops/s (± 5.3%) |
363.76 ops/s (± 7.4%) |
0.98 |
data-client: list-detail-switch-10 |
14.26 ops/s (± 1.2%) |
11.35 ops/s (± 3.4%) |
0.80 |
data-client: update-user-10000 |
103.09 ops/s (± 3.7%) |
99.02 ops/s (± 5.2%) |
0.96 |
data-client: invalidate-and-resolve |
52.08 ops/s (± 4.1%) |
49.75 ops/s (± 3.7%) |
0.96 |
data-client: unshift-item |
285.71 ops/s (± 3.5%) |
277.78 ops/s (± 4.1%) |
0.97 |
data-client: delete-item |
384.62 ops/s (± 6.3%) |
400 ops/s (± 4.3%) |
1.04 |
data-client: move-item |
227.27 ops/s (± 4.4%) |
212.77 ops/s (± 8.5%) |
0.94 |
This comment was automatically generated by workflow using github-action-benchmark.
Contributor
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: d6ce627 | Previous: 869f28f | Ratio |
|---|---|---|---|
normalizeLong |
442 ops/sec (±1.70%) |
451 ops/sec (±1.44%) |
1.02 |
normalizeLong Values |
405 ops/sec (±0.61%) |
417 ops/sec (±0.23%) |
1.03 |
denormalizeLong |
267 ops/sec (±2.74%) |
281 ops/sec (±2.20%) |
1.05 |
denormalizeLong Values |
257 ops/sec (±2.15%) |
251 ops/sec (±2.29%) |
0.98 |
denormalizeLong donotcache |
1006 ops/sec (±0.10%) |
1021 ops/sec (±0.37%) |
1.01 |
denormalizeLong Values donotcache |
729 ops/sec (±0.19%) |
751 ops/sec (±0.19%) |
1.03 |
denormalizeShort donotcache 500x |
1574 ops/sec (±0.13%) |
1585 ops/sec (±0.10%) |
1.01 |
denormalizeShort 500x |
841 ops/sec (±2.13%) |
826 ops/sec (±2.18%) |
0.98 |
denormalizeShort 500x withCache |
6407 ops/sec (±0.27%) |
6196 ops/sec (±0.14%) |
0.97 |
queryShort 500x withCache |
2691 ops/sec (±0.09%) |
2683 ops/sec (±0.17%) |
1.00 |
buildQueryKey All |
53184 ops/sec (±0.35%) |
53826 ops/sec (±0.29%) |
1.01 |
query All withCache |
5890 ops/sec (±0.29%) |
6325 ops/sec (±0.24%) |
1.07 |
denormalizeLong with mixin Entity |
264 ops/sec (±2.26%) |
268 ops/sec (±2.39%) |
1.02 |
denormalizeLong withCache |
6945 ops/sec (±0.16%) |
6214 ops/sec (±0.27%) |
0.89 |
denormalizeLong Values withCache |
5061 ops/sec (±0.17%) |
5006 ops/sec (±0.40%) |
0.99 |
denormalizeLong All withCache |
5726 ops/sec (±0.11%) |
6028 ops/sec (±0.40%) |
1.05 |
denormalizeLong Query-sorted withCache |
5976 ops/sec (±0.15%) |
6277 ops/sec (±0.23%) |
1.05 |
denormalizeLongAndShort withEntityCacheOnly |
1706 ops/sec (±0.19%) |
1630 ops/sec (±0.22%) |
0.96 |
denormalize bidirectional 50 |
2726 ops/sec (±1.87%) |
2706 ops/sec (±1.72%) |
0.99 |
denormalize bidirectional 50 donotcache |
27387 ops/sec (±0.41%) |
27107 ops/sec (±0.61%) |
0.99 |
getResponse |
4484 ops/sec (±0.42%) |
4579 ops/sec (±0.51%) |
1.02 |
getResponse (null) |
9959491 ops/sec (±0.91%) |
10510816 ops/sec (±0.97%) |
1.06 |
getResponse (clear cache) |
261 ops/sec (±2.20%) |
257 ops/sec (±2.26%) |
0.98 |
getSmallResponse |
3279 ops/sec (±0.13%) |
3366 ops/sec (±0.14%) |
1.03 |
getSmallInferredResponse |
2527 ops/sec (±0.09%) |
2483 ops/sec (±0.31%) |
0.98 |
getResponse Collection |
4534 ops/sec (±0.39%) |
4595 ops/sec (±0.38%) |
1.01 |
get Collection |
4542 ops/sec (±0.24%) |
4531 ops/sec (±0.45%) |
1.00 |
get Query-sorted |
5171 ops/sec (±0.16%) |
5174 ops/sec (±0.15%) |
1.00 |
setLong |
445 ops/sec (±0.47%) |
461 ops/sec (±0.24%) |
1.04 |
setLongWithMerge |
249 ops/sec (±0.19%) |
259 ops/sec (±0.25%) |
1.04 |
setLongWithSimpleMerge |
264 ops/sec (±0.14%) |
274 ops/sec (±0.38%) |
1.04 |
setSmallResponse 500x |
945 ops/sec (±0.09%) |
946 ops/sec (±0.13%) |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
Replaced shallow tests with thorough scenario-based tests (27 total): - Round-trip: normalize API data → denormalize parent (Lazy stays raw) → LazyQuery resolves to full entities with all fields checked - Mixed schema: non-Lazy Manager resolves alongside Lazy buildings on same entity; verified instanceof, field values, paths - Dependency tracking: parent paths include Manager but exclude Building; LazyQuery paths include Building PKs but exclude Department - LazyQuery edge cases: subset IDs, empty array, missing entity IDs filtered out, single Entity delegation via Building.queryKey - Memoization isolation: parent ref equality preserved when Building changes; LazyQuery result updates when entity changes; ref equality maintained on unchanged state - Nested Lazy: resolved Building still has its own Lazy rooms as raw IDs; second-level LazyQuery resolves Room entities - Bidirectional Lazy: 1500-node chain no overflow; step-through resolution verifying each level's Lazy field stays raw while resolved entity is correct - Lazy.queryKey returns undefined (not queryable directly) Co-authored-by: natmaster <natmaster@gmail.com>
|
@ntucker Usability concern The
This leaks a performance optimization into the domain model and every component that touches a lazy field. Questions:
|
- Prefix unused params with _ to satisfy @typescript-eslint/no-unused-vars - Fix prettier formatting (auto-fixed via eslint --fix) - Fix import order in schema.d.ts - Remove unused imports in test file - Add changeset for @data-client/endpoint, rest, graphql (minor) Co-authored-by: natmaster <natmaster@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3829 +/- ##
==========================================
- Coverage 98.07% 98.05% -0.03%
==========================================
Files 151 152 +1
Lines 2855 2872 +17
Branches 561 563 +2
==========================================
+ Hits 2800 2816 +16
- Misses 11 12 +1
Partials 44 44 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
…, Values) When the inner schema is an explicit class instance (e.g. new schema.Array(Building)), LazyQuery.queryKey would delegate to the inner schema's queryKey which always returns undefined for Array and Values schemas. This caused MemoCache.query to short-circuit and return no data, because the args[0] fallback was never reached. Fix: only delegate to inner schema's queryKey when schema.key exists (Entity, Collection), which distinguishes schemas with meaningful queryKey logic from container schemas (Array, Values) that have no-op stubs. Also fixes pre-existing TypeScript errors in Lazy.test.ts and adds tests for explicit schema.Array and schema.Values inner schemas. Co-authored-by: Nathaniel Tucker <me@ntucker.me>
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
3 participants
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.
Motivation
LazyQuery.queryKeycheckstypeof schema.queryKey === 'function'and delegates to the inner schema'squeryKeyif it exists. However, some schema classes likeschema.Arrayandschema.Valueshave aqueryKeymethod that always returnsundefined. When the inner schema is an explicit class instance (e.g.,new schema.Array(Building)instead of shorthand[Building]), the delegation producesundefined, causingMemoCache.queryto short-circuit and return no data. Theargs[0]fallback -- which is the correct behavior for these schemas -- is never reached.Solution
The fix adds a
schema.keycheck alongside thetypeof schema.queryKey === 'function'check inLazyQuery.queryKey. Only schemas with both aqueryKeymethod and akeyproperty (Entity, Collection) will have theirqueryKeydelegated to. Container schemas likeArrayandValues(which lackkey) fall through to theargs[0]passthrough.This approach:
undefined)schema.Array(Entity)andschema.Values(Entity)usage with LazyAlso fixes pre-existing TypeScript errors in
Lazy.test.tsand adds test coverage for explicitschema.Arrayandschema.Valuesinner schemas including normalize, denormalize, and LazyQuery resolution.Open questions
None.