Skip to content

enhance: ImmutableJS always denormalizes to normal JS #3421

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

Merged
merged 1 commit into from
Mar 20, 2025

Conversation

ntucker
Copy link
Collaborator

@ntucker ntucker commented Mar 18, 2025

BREAKING CHANGE: Denormalize always transforms immutablejs entities into the class

Previously using ImmutableJS structures when calling denormalize() would maintain
nested schemas as immutablejs structures still. Now everything is converted to normal JS.
This is how the types have always been specified.

Motivation

Think through use of immutablejs. Maximize performance and compatibility.

Solution

Don't check objects are immutable everywhere.

Next Work

ideal case:

  • immutablejs used for lookup table. but normalize does not turn entitys or such into immutablejs themselves
  • then when denormalizing on normal path, no conversion needs to be done
  • collections should normalize to immutablelist as they like to be mutated and potentially are long

also support immutablejs at entity level

  • have special immutablejsentity (maybe RecordEntity or such) that will keep immutable structures in normalization and denormalization stages. this will have special code
  • since this will assume records, we don't even need to convert it to js to call on the lifecycles as they expect immutablejs structures

then we can remove all immutable checks beyond getEntity

Copy link

changeset-bot bot commented Mar 18, 2025

🦋 Changeset detected

Latest commit: 546443d

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

This PR includes changesets to release 11 packages
Name Type
@data-client/normalizr Major
@data-client/core Major
example-benchmark Patch
normalizr-github-example Patch
normalizr-redux-example Patch
normalizr-relationships Patch
@data-client/react Major
@data-client/img Major
@data-client/test Major
test-bundlesize Patch
coinbase-lite 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

Copy link
Contributor

github-actions bot commented Mar 18, 2025

Size Change: -7 B (-0.01%)

Total Size: 76.8 kB

ℹ️ View Unchanged
Filename Size Change
examples/test-bundlesize/dist/App.js 3.41 kB 0 B
examples/test-bundlesize/dist/polyfill.js 311 B 0 B
examples/test-bundlesize/dist/rdc.js 15.1 kB -7 B (-0.05%)
examples/test-bundlesize/dist/react.js 57.3 kB 0 B
examples/test-bundlesize/dist/webpack-runtime.js 726 B 0 B

compressed-size-action

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: 546443d Previous: 397c113 Ratio
normalizeLong 540 ops/sec (±0.22%) 540 ops/sec (±0.46%) 1
infer All 9094 ops/sec (±1.12%) 8900 ops/sec (±1.43%) 0.98
denormalizeLong 273 ops/sec (±3.42%) 248 ops/sec (±3.49%) 0.91
denormalizeLong donotcache 966 ops/sec (±0.31%) 893 ops/sec (±0.37%) 0.92
denormalizeShort donotcache 500x 1410 ops/sec (±0.20%) 1403 ops/sec (±0.26%) 1.00
denormalizeShort 500x 785 ops/sec (±2.26%) 770 ops/sec (±2.14%) 0.98
denormalizeShort 500x withCache 5182 ops/sec (±0.27%) 5522 ops/sec (±0.26%) 1.07
denormalizeLong with mixin Entity 264 ops/sec (±1.93%) 245 ops/sec (±2.22%) 0.93
denormalizeLong withCache 6963 ops/sec (±0.32%) 6776 ops/sec (±0.19%) 0.97
denormalizeLong All withCache 7643 ops/sec (±0.27%) 8278 ops/sec (±0.22%) 1.08
denormalizeLong Query-sorted withCache 7323 ops/sec (±0.68%) 7744 ops/sec (±1.28%) 1.06
denormalizeLongAndShort withEntityCacheOnly 1754 ops/sec (±0.29%) 1692 ops/sec (±0.39%) 0.96
getResponse 7676 ops/sec (±1.14%) 7172 ops/sec (±1.15%) 0.93
getResponse (null) 4710107 ops/sec (±0.46%) 4953127 ops/sec (±0.42%) 1.05
getResponse (clear cache) 269 ops/sec (±1.83%) 250 ops/sec (±2.25%) 0.93
getSmallResponse 2717 ops/sec (±0.07%) 2815 ops/sec (±0.23%) 1.04
getSmallInferredResponse 2006 ops/sec (±0.31%) 2047 ops/sec (±0.28%) 1.02
getResponse Query-sorted 7857 ops/sec (±0.41%) 7300 ops/sec (±0.48%) 0.93
getResponse Collection 6617 ops/sec (±1.09%) 6492 ops/sec (±1.00%) 0.98
get Collection 5781 ops/sec (±0.47%) 5712 ops/sec (±0.24%) 0.99
setLong 535 ops/sec (±0.34%) 541 ops/sec (±0.31%) 1.01
setLongWithMerge 234 ops/sec (±0.35%) 230 ops/sec (±0.39%) 0.98
setLongWithSimpleMerge 250 ops/sec (±0.30%) 246 ops/sec (±0.25%) 0.98
setSmallResponse 500x 927 ops/sec (±0.47%) 915 ops/sec (±0.29%) 0.99

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

Copy link

codecov bot commented Mar 18, 2025

Codecov Report

Attention: Patch coverage is 93.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.74%. Comparing base (397c113) to head (546443d).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
packages/normalizr/src/denormalize/unvisit.ts 93.10% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3421   +/-   ##
=======================================
  Coverage   98.74%   98.74%           
=======================================
  Files         136      136           
  Lines        2396     2399    +3     
  Branches      490      489    -1     
=======================================
+ Hits         2366     2369    +3     
  Misses         16       16           
  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.

@ntucker ntucker force-pushed the immutable-denormalize branch 14 times, most recently from 022c491 to 19580c4 Compare March 20, 2025 08:51
@ntucker ntucker force-pushed the immutable-denormalize branch 5 times, most recently from 87eadd1 to 6360342 Compare March 20, 2025 10:00
@ntucker ntucker force-pushed the immutable-denormalize branch from 6360342 to 546443d Compare March 20, 2025 10:46
@ntucker ntucker merged commit 246cde6 into master Mar 20, 2025
22 of 23 checks passed
@ntucker ntucker deleted the immutable-denormalize branch March 20, 2025 12:17
@github-actions github-actions bot mentioned this pull request Mar 20, 2025
@github-actions github-actions bot mentioned this pull request May 1, 2025
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