Skip to content

enhance: ImmutableJS support moved to /immutable export #3468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ntucker
Copy link
Collaborator

@ntucker ntucker commented Apr 10, 2025

BREAKING CHANGE: To use

Follow up to #3449

Motivation

Solution

  • delegate.getEntities()
  • new MemoCache(ImmutableDelegate)
Benchmark suite Current: cb90a4f Previous: 79f4eab Ratio
denormalizeShort 500x withCache 6397 ops/sec (±0.32%) 5157 ops/sec (±0.25%) 0.81
queryShort 500x withCache 2871 ops/sec (±0.33%) 2403 ops/sec (±0.26%) 0.84
getResponse (null) 7185893 ops/sec (±0.91%) 5527559 ops/sec (±0.43%) 0.77
getSmallResponse 3138 ops/sec (±0.29%) 2800 ops/sec (±0.28%) 0.89
getSmallInferredResponse 2398 ops/sec (±0.25%) 2089 ops/sec (±0.35%) 0.87

Copy link

changeset-bot bot commented Apr 10, 2025

⚠️ No Changeset found

Latest commit: f3f1824

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Apr 10, 2025

Size Change: -94 B (-0.12%)

Total Size: 77.6 kB

Filename Size Change
examples/test-bundlesize/dist/rdcClient.js 10.1 kB -99 B (-0.97%)
ℹ️ 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.6 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

Copy link

codecov bot commented Apr 10, 2025

Codecov Report

Attention: Patch coverage is 98.96907% with 1 line in your changes missing coverage. Please review.

Project coverage is 98.75%. Comparing base (e15d2af) to head (a477670).

Files with missing lines Patch % Lines
packages/core/src/state/GCPolicy.ts 90.90% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3468   +/-   ##
=======================================
  Coverage   98.74%   98.75%           
=======================================
  Files         124      128    +4     
  Lines        2239     2247    +8     
  Branches      460      455    -5     
=======================================
+ Hits         2211     2219    +8     
  Misses         13       13           
  Partials       15       15           

☔ 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.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: 0115996 Previous: 79f4eab Ratio
normalizeLong 529 ops/sec (±1.38%) 541 ops/sec (±1.19%) 1.02
infer All 9528 ops/sec (±1.32%) 10013 ops/sec (±0.44%) 1.05
denormalizeLong 294 ops/sec (±2.83%) 291 ops/sec (±2.32%) 0.99
denormalizeLong donotcache 1062 ops/sec (±0.73%) 1054 ops/sec (±0.34%) 0.99
denormalizeShort donotcache 500x 1571 ops/sec (±0.26%) 1534 ops/sec (±0.38%) 0.98
denormalizeShort 500x 861 ops/sec (±1.92%) 834 ops/sec (±1.74%) 0.97
denormalizeShort 500x withCache 6377 ops/sec (±0.37%) 5157 ops/sec (±0.25%) 0.81
queryShort 500x withCache 2776 ops/sec (±0.25%) 2403 ops/sec (±0.26%) 0.87
denormalizeLong with mixin Entity 365 ops/sec (±0.43%) 284 ops/sec (±1.92%) 0.78
denormalizeLong withCache 7067 ops/sec (±0.27%) 8214 ops/sec (±0.25%) 1.16
denormalizeLong All withCache 6687 ops/sec (±0.28%) 7313 ops/sec (±0.22%) 1.09
denormalizeLong Query-sorted withCache 6779 ops/sec (±0.21%) 7268 ops/sec (±0.25%) 1.07
denormalizeLongAndShort withEntityCacheOnly 1803 ops/sec (±0.29%) 1777 ops/sec (±0.50%) 0.99
getResponse 5874 ops/sec (±1.50%) 5642 ops/sec (±1.49%) 0.96
getResponse (null) 6834208 ops/sec (±0.78%) 5527559 ops/sec (±0.43%) 0.81
getResponse (clear cache) 359 ops/sec (±0.36%) 273 ops/sec (±2.00%) 0.76
getSmallResponse 3095 ops/sec (±0.37%) 2800 ops/sec (±0.28%) 0.90
getSmallInferredResponse 2392 ops/sec (±0.45%) 2089 ops/sec (±0.35%) 0.87
getResponse Collection 5689 ops/sec (±1.01%) 5753 ops/sec (±1.50%) 1.01
get Collection 5712 ops/sec (±0.52%) 5624 ops/sec (±0.87%) 0.98
get Query-sorted 5655 ops/sec (±0.35%) 6852 ops/sec (±0.27%) 1.21
setLong 557 ops/sec (±0.28%) 544 ops/sec (±0.17%) 0.98
setLongWithMerge 243 ops/sec (±0.54%) 248 ops/sec (±0.37%) 1.02
setLongWithSimpleMerge 258 ops/sec (±0.36%) 256 ops/sec (±0.30%) 0.99
setSmallResponse 500x 956 ops/sec (±0.25%) 944 ops/sec (±0.37%) 0.99

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

@ntucker ntucker force-pushed the imm-denorm branch 11 times, most recently from 36683fd to a477670 Compare April 15, 2025 16:21
@ntucker ntucker force-pushed the imm-denorm branch 2 times, most recently from ccd6657 to 0e30d7c Compare April 28, 2025 04:39
@ntucker ntucker requested a review from Copilot May 1, 2025 03:59
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances ImmutableJS support by updating various components to use new Delegate patterns and adjusting related API calls. Key changes include:

  • Consolidating type imports and modifying local cache retrievals.
  • Replacing direct entity retrieval calls with new delegate calls in both normalization and denormalization.
  • Updating tests to use the new delegate parameters for MemoCache queries.

Reviewed Changes

Copilot reviewed 41 out of 42 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/normalizr/src/denormalize/localCache.ts Adjusted import refinements and non-null assertion on cache retrieval.
packages/normalizr/src/denormalize/getEntities.ts Updated delegate usage and return type to support symbol keys.
packages/normalizr/src/denormalize/denormalize.ts Replaced getEntities call with PlainDelegate and removed isImmutable flag from getUnvisit.
packages/normalizr/src/denormalize/denormalize.immutable.ts Added new immutable-specific denormalization implementation.
packages/normalizr/src/denormalize/cache.ts Simplified type imports.
packages/normalizr/src/denormalize/UNDEF.ts Updated UNDEF constant with type assertion to any.
Various test files Updated MemoCache instantiation and delegate usage in tests.
packages/endpoint and packages/core files Refactored entity lookup and GCPolicy iteration using destructuring for clarity.
packages/endpoint/src/interface.ts Expanded IQueryDelegate and INormalizeDelegate definitions to include getEntities.
packages/endpoint/src/index.ts Re-exported all interface types for consistency.
Files not reviewed (1)
  • packages/normalizr/package.json: Language not supported
Comments suppressed due to low confidence (1)

packages/normalizr/src/denormalize/denormalize.ts:19

  • The removal of the isImmutable(entities) argument in the getUnvisit call may alter the behavior if getUnvisit depends on this flag for handling immutable data. Please verify that this change is intentional and that getUnvisit no longer requires an explicit immutable indicator.
return getUnvisit(PlainDelegate.forDenorm(entities), new LocalCache(), args, )(schema, input).data;

@ntucker ntucker force-pushed the imm-denorm branch 2 times, most recently from 05316f7 to d1db752 Compare May 22, 2025 15:30
@ntucker ntucker force-pushed the imm-denorm branch 3 times, most recently from cb90a4f to f3f1824 Compare June 1, 2025 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant