enhance: Add consolidate callbacks for queryKey and normalize in 'delegates' #3449

ntucker · 2025-04-07T16:59:37Z

Motivation

Two ImmutableJS usage patterns supported:

Lookup tables only
{ entities: Map<string, Map<string, object>>, indexes: Map<string, Map<string, Map<string, string>>> }
Support in normalized form of data itself (represented by schemas)
{ entities: Map<string, Map<string, Immutablejs.Record>> }

Goal:

Remove unnecessary checks to improve performance
Reduce bundled size: Allow for conditional code inclusion based on usage patterns
Ensure full support of these use cases, including tests. (currently some things like schema.All assumed POJOs)

Icing on the cake

Removing the hard merge interface from normalization means we can finally implement more powerful special cases - like Collection.insertAt(), Collection.add (sorted). Future work we will add the snapshot interface (of denormalization) to normalize delegate to make insertion in sorted order possible.

Solution

Hoist ImmutableJS detection to initialization rather than runtime since people don’t typically switch between the two. In other words, import distinct normalization functions like normalize, denormalize, MemoCache.

For this PR we will simply hoist detection to the top level functions and keep their interfaces the same. We focus instead of the design of recursion methods.

New interfaces:

IQueryDelegate
INormalizeDelegate
Mergeable

Delegates

We consolidate all 'callback' functions during recursion calls into a single 'delegate' argument. This only applies to normalize and queryKey since denormalize does not need any callbacks.

Schema changes:

normalize(input, parent, key, args, visit, addEntity, getEntity, checkLoop) -> normalize(input, parent, key, args, visit, delegate)
queryKey(args, queryKey, getEntity, getIndex) -> queryKey(args, unvisit, delegate)
- Recursive call renamed queryKey -> unvisit to be consistent with denormalize/normalize nomenclature
- unvisit only requires arguments of its recursive context (meaning drop the getEntity, getIndex args, and we don't need delegate here either)
  queryKey(this.schema, args, getEntity, getIndex) -> unvisit(this.schema, args)

/** Accessors to the currently processing state while building query */
export interface IQueryDelegate {
  getEntity: GetEntity;
  getIndex: GetIndex;
}

/** Helpers during schema.normalize() */
export interface INormalizeDelegate {
  /** Action meta-data for this normalize call */
  readonly meta: { fetchedAt: number; date: number; expiresAt: number };
  /** Gets any previously normalized entity from store */
  getEntity: GetEntity;
  /** Updates an entity using merge lifecycles when it has previously been set */
  mergeEntity(
    schema: Mergeable & { indexes?: any },
    pk: string,
    incomingEntity: any,
  ): void;
  /** Sets an entity overwriting any previously set values */
  setEntity(
    schema: { key: string; indexes?: any },
    pk: string,
    entity: any,
    meta?: { fetchedAt: number; date: number; expiresAt: number },
  ): void;
  /** Returns true when we're in a cycle, so we should not continue recursing */
  checkLoop(key: string, pk: string, input: object): boolean;
}

Advantages

Easier to avoid breakages when changing these
Less arguments on stack means better performance
Using classes makes it easy to provide Immutable + POJO implementations
Classes also make JIT possibilities better as consolidated data construction results in less ICs (therefore more monomorphic paths).

Mergeable

We have reduced the scope of EntityInterface required elements to createIfValid, pk, key

We now have an addition interface Mergeable, which is used during normalization with delegate.mergeEntity.

export interface Mergeable {
  key: string;
  merge(existing: any, incoming: any): any;
  mergeWithStore(
    existingMeta: any,
    incomingMeta: any,
    existing: any,
    incoming: any,
  ): any;
  mergeMetaWithStore(
    existingMeta: any,
    incomingMeta: any,
    existing: any,
    incoming: any,
  ): any;
}

This means that if you don't call that, you won't need to implement merges, as you can simply set it directly.

Additionally, for cases that need additional context (like Collection.insertAt or Collection.add) - we can dynamically construct the Mergeable in normalize, using closures to access that context. This means we don't pass the schema itself but another Object.

Other changes included

Fix

ensure string id in Entity set when process returns undefined (meaning INVALID)
Controller.get and Controller.getQueryMeta state types

Follow ups

BREAKING: Have distinct imports for immutable or non-immutable versions.
Add Collection.insertAt and Collection.add (sorted).
schema.All should not assume POJOs. We need to change this interface perhaps.

Performance improvements (approx)

queryShort 500x withCache: 5%
denormalizeShort 500x withCache 3%
denormalizeShort 500x withCache 8%
denormalizeLong 4%

…ternally using the delegate

changeset-bot · 2025-04-07T16:59:41Z

🦋 Changeset detected

Latest commit: 8b214df

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 14 packages

Name	Type
@data-client/core	Major
@data-client/react	Major
@data-client/endpoint	Major
@data-client/graphql	Major
@data-client/rest	Major
@data-client/normalizr	Major
example-benchmark	Patch
@data-client/img	Major
@data-client/test	Major
test-bundlesize	Patch
coinbase-lite	Patch
normalizr-github-example	Patch
normalizr-redux-example	Patch
normalizr-relationships	Patch

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

codecov · 2025-04-07T17:00:59Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.79%. Comparing base (fdfb34d) to head (8b214df).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3449      +/-   ##
==========================================
+ Coverage   98.78%   98.79%   +0.01%     
==========================================
  Files         123      125       +2     
  Lines        2221     2245      +24     
  Branches      462      462              
==========================================
+ Hits         2194     2218      +24     
  Misses         13       13              
  Partials       14       14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2025-04-07T17:01:23Z

Size Change: +79 B (+0.1%)

Total Size: 77.8 kB

Filename	Size	Change
`examples/test-bundlesize/dist/rdcClient.js`	10.3 kB	+84 B (+0.83%)

ℹ️ View Unchanged

Filename	Size	Change
`examples/test-bundlesize/dist/App.js`	3.42 kB	0 B
`examples/test-bundlesize/dist/polyfill.js`	311 B	0 B
`examples/test-bundlesize/dist/rdcEndpoint.js`	5.65 kB	-5 B (-0.09%)
`examples/test-bundlesize/dist/react.js`	57.5 kB	0 B
`examples/test-bundlesize/dist/webpack-runtime.js`	726 B	0 B

_{compressed-size-action}

github-actions

Benchmark

Benchmark suite	Current: `8b214df`	Previous: `ca4536f`	Ratio
`normalizeLong`	`504` ops/sec (`±0.53%`)	`504` ops/sec (`±0.55%`)	`1`
`infer All`	`9099` ops/sec (`±1.11%`)	`9209` ops/sec (`±0.86%`)	`1.01`
`denormalizeLong`	`275` ops/sec (`±3.13%`)	`266` ops/sec (`±2.80%`)	`0.97`
`denormalizeLong donotcache`	`987` ops/sec (`±0.42%`)	`985` ops/sec (`±0.34%`)	`1.00`
`denormalizeShort donotcache 500x`	`1418` ops/sec (`±0.14%`)	`1389` ops/sec (`±0.68%`)	`0.98`
`denormalizeShort 500x`	`786` ops/sec (`±1.88%`)	`762` ops/sec (`±2.30%`)	`0.97`
`denormalizeShort 500x withCache`	`5260` ops/sec (`±0.19%`)	`5369` ops/sec (`±0.33%`)	`1.02`
`queryShort 500x withCache`	`2427` ops/sec (`±0.34%`)	`2407` ops/sec (`±0.31%`)	`0.99`
`denormalizeLong with mixin Entity`	`263` ops/sec (`±1.95%`)	`256` ops/sec (`±1.95%`)	`0.97`
`denormalizeLong withCache`	`6911` ops/sec (`±0.22%`)	`6848` ops/sec (`±0.20%`)	`0.99`
`denormalizeLong All withCache`	`8230` ops/sec (`±0.19%`)	`8080` ops/sec (`±0.28%`)	`0.98`
`denormalizeLong Query-sorted withCache`	`7567` ops/sec (`±1.04%`)	`7845` ops/sec (`±0.54%`)	`1.04`
`denormalizeLongAndShort withEntityCacheOnly`	`1715` ops/sec (`±0.42%`)	`1634` ops/sec (`±0.35%`)	`0.95`
`getResponse`	`6218` ops/sec (`±0.89%`)	`6099` ops/sec (`±1.25%`)	`0.98`
`getResponse (null)`	`6086342` ops/sec (`±0.66%`)	`5782251` ops/sec (`±0.59%`)	`0.95`
`getResponse (clear cache)`	`264` ops/sec (`±1.58%`)	`248` ops/sec (`±2.17%`)	`0.94`
`getSmallResponse`	`2626` ops/sec (`±0.22%`)	`2547` ops/sec (`±0.28%`)	`0.97`
`getSmallInferredResponse`	`2051` ops/sec (`±0.34%`)	`2071` ops/sec (`±0.28%`)	`1.01`
`getResponse Collection`	`6991` ops/sec (`±1.00%`)	`6571` ops/sec (`±1.24%`)	`0.94`
`get Collection`	`6141` ops/sec (`±0.32%`)	`6082` ops/sec (`±0.55%`)	`0.99`
`get Query-sorted`	`6581` ops/sec (`±0.30%`)	`6659` ops/sec (`±0.27%`)	`1.01`
`setLong`	`505` ops/sec (`±0.37%`)	`523` ops/sec (`±0.26%`)	`1.04`
`setLongWithMerge`	`228` ops/sec (`±0.27%`)	`230` ops/sec (`±0.25%`)	`1.01`
`setLongWithSimpleMerge`	`240` ops/sec (`±0.58%`)	`242` ops/sec (`±0.26%`)	`1.01`
`setSmallResponse 500x`	`914` ops/sec (`±0.35%`)	`906` ops/sec (`±0.31%`)	`0.99`

This comment was automatically generated by workflow using github-action-benchmark.

ntucker added 12 commits April 7, 2025 13:01

enhance: Immutable changes

9bfcbd0

enhance: Adding class

fd3617c

enhance: Classes for query compute

a0b63b0

enhance: Normalize snapshots

a0dbbde

enhance: Remove merge from interface and let entities handle logic in…

4e4e6e0

…ternally using the delegate

enhance: Share merge logic with entity and collection

6a43fe2

internal: Fix tests

4031a14

enhance: Remove merge logic sharing between Entity and Collection

ac3a490

enhance: Update downtypes

b6452e4

enhance: Remove controller getExpiryStatus (for now)

9754ac8

docs: Add changelogs

69dbb73

internal: Remove unused code

8b214df

github-actions bot reviewed Apr 7, 2025

View reviewed changes

ntucker merged commit 1f491a9 into master Apr 7, 2025
24 checks passed

ntucker deleted the immutable branch April 7, 2025 17:03

github-actions bot mentioned this pull request Apr 7, 2025

Version Packages #3425

Open

ntucker mentioned this pull request Apr 10, 2025

enhance: Hoist ImmutableJS vs Pojo split to import-time #3468

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

enhance: Add consolidate callbacks for queryKey and normalize in 'delegates' #3449

enhance: Add consolidate callbacks for queryKey and normalize in 'delegates' #3449

Uh oh!

ntucker commented Apr 7, 2025 •

edited

Loading

Uh oh!

changeset-bot bot commented Apr 7, 2025

Uh oh!

codecov bot commented Apr 7, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Apr 7, 2025

Uh oh!

github-actions bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

enhance: Add consolidate callbacks for queryKey and normalize in 'delegates' #3449

enhance: Add consolidate callbacks for queryKey and normalize in 'delegates' #3449

Uh oh!

Conversation

ntucker commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Solution

Delegates

Mergeable

Other changes included

Follow ups

Performance improvements (approx)

Uh oh!

changeset-bot bot commented Apr 7, 2025

🦋 Changeset detected

Uh oh!

codecov bot commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Apr 7, 2025

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Benchmark

Uh oh!

Uh oh!

Uh oh!

ntucker commented Apr 7, 2025 •

edited

Loading

codecov bot commented Apr 7, 2025 •

edited

Loading