feat: Evict ImmutableJS support from main bundles behind /imm value policy#4018
Open
ntucker wants to merge 5 commits into
Open
feat: Evict ImmutableJS support from main bundles behind /imm value policy#4018ntucker wants to merge 5 commits into
ntucker wants to merge 5 commits into
Conversation
…olicy Immutable Plan Part 1 (plans/immutablejs-support.md): immediate /imm LocalCache(args) fix plus Stage F1. - Fix /imm denormalize dropping args (Scalar/argsKey schemas) - Value policy seam: getUnvisit(valuePolicy) -> delegate.unvisitObject/getField - Delete isImmutable branches from normalizr Object, endpoint Object+Polymorphic - Consolidate drifted ImmutableUtils copies; propagate first symbol - Dev-mode loud error on immutable input at main-entry denormalize - Cross-version capability fallback + legacyDelegate tests - Micro benchmark cases for object/polymorphic denormalize (~12% object win) - CI guard: no __ownerID marker in production bundles
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: 007855c The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
Contributor
|
Size Change: -197 B (-0.24%) Total Size: 80.9 kB 📦 View Changed
ℹ️ 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: 007855c | Previous: 78d8a30 | Ratio |
|---|---|---|---|
data-client: getlist-100 |
142.86 ops/s (± 5.7%) |
135.14 ops/s (± 4.3%) |
0.95 |
data-client: getlist-500 |
43.29 ops/s (± 4.9%) |
46.73 ops/s (± 3.9%) |
1.08 |
data-client: update-entity |
370.37 ops/s (± 10.2%) |
363.76 ops/s (± 9.2%) |
0.98 |
data-client: update-user |
370.37 ops/s (± 8.3%) |
350.99 ops/s (± 8.0%) |
0.95 |
data-client: getlist-500-sorted |
46.19 ops/s (± 8.1%) |
47.74 ops/s (± 8.3%) |
1.03 |
data-client: update-entity-sorted |
327.96 ops/s (± 8.5%) |
303.03 ops/s (± 6.2%) |
0.92 |
data-client: update-entity-multi-view |
385.19 ops/s (± 7.7%) |
344.83 ops/s (± 6.4%) |
0.90 |
data-client: list-detail-switch-10 |
10.71 ops/s (± 6.4%) |
8.95 ops/s (± 9.3%) |
0.84 |
data-client: update-user-10000 |
74.07 ops/s (± 16.0%) |
98.04 ops/s (± 7.5%) |
1.32 |
data-client: invalidate-and-resolve |
37.17 ops/s (± 4.7%) |
39.53 ops/s (± 5.8%) |
1.06 |
data-client: unshift-item |
215.08 ops/s (± 6.5%) |
227.27 ops/s (± 5.0%) |
1.06 |
data-client: delete-item |
294.12 ops/s (± 4.2%) |
294.12 ops/s (± 4.0%) |
1 |
data-client: move-item |
177.12 ops/s (± 8.9%) |
188.68 ops/s (± 9.1%) |
1.07 |
This comment was automatically generated by workflow using github-action-benchmark.
Contributor
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: 007855c | Previous: f3b96b0 | Ratio |
|---|---|---|---|
normalizeLong |
436 ops/sec (±3.98%) |
443 ops/sec (±3.48%) |
1.02 |
normalizeLong Values |
404 ops/sec (±1.27%) |
407 ops/sec (±0.23%) |
1.01 |
normalizeLong Scalar |
334 ops/sec (±0.99%) |
341 ops/sec (±0.76%) |
1.02 |
normalizeLong Scalar update |
844 ops/sec (±0.66%) |
845 ops/sec (±1.61%) |
1.00 |
denormalizeLong |
238 ops/sec (±4.98%) |
241 ops/sec (±4.62%) |
1.01 |
denormalizeLong Values |
221 ops/sec (±3.95%) |
225 ops/sec (±3.87%) |
1.02 |
denormalizeLong donotcache |
957 ops/sec (±0.32%) |
960 ops/sec (±0.24%) |
1.00 |
denormalizeLong Values donotcache |
730 ops/sec (±0.15%) |
712 ops/sec (±0.50%) |
0.98 |
denormalizeLong Scalar donotcache |
912 ops/sec (±0.38%) |
888 ops/sec (±0.14%) |
0.97 |
denormalizeShort donotcache 500x |
1514 ops/sec (±0.11%) |
1548 ops/sec (±0.20%) |
1.02 |
denormalizeShort 500x |
685 ops/sec (±4.52%) |
715 ops/sec (±3.72%) |
1.04 |
denormalizeShort 500x withCache |
6507 ops/sec (±0.15%) |
6610 ops/sec (±0.15%) |
1.02 |
queryShort 500x withCache |
2677 ops/sec (±0.10%) |
2879 ops/sec (±0.14%) |
1.08 |
buildQueryKey All |
52966 ops/sec (±0.66%) |
53962 ops/sec (±0.51%) |
1.02 |
query All withCache |
5816 ops/sec (±0.64%) |
6583 ops/sec (±0.27%) |
1.13 |
denormalizeLong with mixin Entity |
233 ops/sec (±3.48%) |
238 ops/sec (±4.06%) |
1.02 |
denormalizeLong withCache |
6898 ops/sec (±0.28%) |
7328 ops/sec (±0.15%) |
1.06 |
denormalizeLong withCache (Scalar churn) |
6880 ops/sec (±0.21%) |
7244 ops/sec (±0.18%) |
1.05 |
denormalizeLong Values withCache |
5218 ops/sec (±0.13%) |
4872 ops/sec (±0.45%) |
0.93 |
denormalizeLong Scalar withCache |
7784 ops/sec (±1.67%) |
7618 ops/sec (±1.96%) |
0.98 |
denormalizeLong Scalar update withCache |
4073 ops/sec (±0.16%) |
4054 ops/sec (±0.38%) |
1.00 |
denormalizeLong All withCache |
6365 ops/sec (±0.20%) |
6327 ops/sec (±0.23%) |
0.99 |
denormalizeLong Query-sorted withCache |
5866 ops/sec (±0.19%) |
6633 ops/sec (±0.19%) |
1.13 |
denormalizeLongAndShort withEntityCacheOnly |
1691 ops/sec (±0.27%) |
1720 ops/sec (±0.40%) |
1.02 |
denormalize bidirectional 50 |
4796 ops/sec (±5.18%) |
4913 ops/sec (±4.55%) |
1.02 |
denormalize bidirectional 50 donotcache |
40122 ops/sec (±0.32%) |
40638 ops/sec (±0.26%) |
1.01 |
getResponse |
4620 ops/sec (±1.04%) |
4558 ops/sec (±0.83%) |
0.99 |
getResponse (null) |
10108052 ops/sec (±1.09%) |
9973267 ops/sec (±0.75%) |
0.99 |
getResponse (clear cache) |
218 ops/sec (±4.35%) |
227 ops/sec (±4.05%) |
1.04 |
getSmallResponse |
3446 ops/sec (±0.08%) |
3441 ops/sec (±0.27%) |
1.00 |
getSmallInferredResponse |
2587 ops/sec (±0.25%) |
2661 ops/sec (±0.11%) |
1.03 |
getResponse Collection |
4540 ops/sec (±0.68%) |
4475 ops/sec (±0.40%) |
0.99 |
get Collection |
4384 ops/sec (±0.31%) |
4346 ops/sec (±0.47%) |
0.99 |
get Query-sorted |
5209 ops/sec (±0.47%) |
5190 ops/sec (±0.23%) |
1.00 |
setLong |
450 ops/sec (±0.14%) |
451 ops/sec (±0.78%) |
1.00 |
setLongWithMerge |
252 ops/sec (±0.26%) |
252 ops/sec (±0.20%) |
1 |
setLongWithSimpleMerge |
257 ops/sec (±0.77%) |
267 ops/sec (±0.79%) |
1.04 |
setSmallResponse 500x |
889 ops/sec (±1.40%) |
906 ops/sec (±1.34%) |
1.02 |
This comment was automatically generated by workflow using github-action-benchmark.
- unvisit shorthand dispatch calls valuePolicy.denormalizeObject directly (delegate hop only needed for cross-package callers) - endpoint Object fallback loop: early-return symbol before assignment, matching normalizr ordering to prevent re-drift - Polymorphic: hoist delegate.getField once per denormalizeValue - main denormalize entry passes MemoPolicy.valuePolicy explicitly (symmetry with /imm entry) - recursive grep in immutable-leak CI guard - trim redundant comments
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4018 +/- ##
==========================================
+ Coverage 98.21% 98.41% +0.19%
==========================================
Files 154 155 +1
Lines 3024 3024
Branches 601 604 +3
==========================================
+ Hits 2970 2976 +6
+ Misses 11 9 -2
+ Partials 43 39 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…_ENV stripping - normalizr rollup: replace process.env.NODE_ENV in production browser build (matches endpoint) so dev-only guards are stripped from prebuilt dist - micro: add MemoCache-hit counterparts for object/union denormalize to guard per-call overhead on the cached path - tests: 100% branch coverage on all F1-touched files — Record (v4 _map and v5 shapes), null-prototype and falsy inputs, custom MemoCache policy without valuePolicy, Union passthrough inference, primitive store values
- Fact-checked Record internals against real immutable v3/v4/v5: the _map.__ownerID detection branch is for v3 Records (v4+ Records carry their own __ownerID) — corrected mislabeled v4 comments in tests and the isImmutable doc - Reframe falsy-input tests as the actual invariant (not misdetected as immutable) with GIGO output documented as pre-existing behavior rather than a bare characterization - Move custom-policy compat test out of parametrized block (ran twice, ignored parameters); assert instance type not just field value - Record-wrapper test now asserts container preservation (instanceof)
isImmutable and the dev-mode input guards now check only the own __ownerID property (present on all v4-v5 collections and Records). The _map.__ownerID fallback existed solely for v3 Records (2018). Changeset notes v4+ requirement; README states supported range.
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
1 participant
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.
Fixes # .
Motivation
First part of the ImmutableJS two-track plan: the immediate
/immLocalCache(args)bug fix plus Stage F1 (evict immutable code from main bundles). Main-entry denormalize paid a per-objectisImmutableduck-type check for every user, immutable support code shipped in every bundle, and the twoImmutableUtils.tscopies (normalizr/endpoint) had drifted semantics. Additionally,denormalizefrom@data-client/normalizr/immsilently droppedargs, breaking args-dependent schemas likeScalar.Solution
Immediate fix:
denormalize.imm.tsnow constructsnew LocalCache(args)like the main entry; regression test verified to fail without the fix.Value-policy seam (F0.2 design recorded in the plan doc): the value representation is resolved once at
getUnvisitconstruction, never per call:getUnvisit(getEntity, cache, args, valuePolicy = PlainValuePolicy); the delegate gains two always-present members (single hidden class, no optional fields):unvisitObject(schema, input)for object-shaped schema nodes andgetField(value, key)for polymorphicschema/iddiscriminator reads.PlainValuePolicy(main entries) runs the POJO loop with zero checks;ImmValuePolicy(/immonly) adds the ImmutableJS branch.MemoCacheforwards an optionalpolicy.valuePolicy, so external custom policies keep working unchanged.legacyDelegate.test.ts.Polymorphic.denormalizeValue(value, unvisit)→denormalizeValue(value, delegate)(noted in changeset).Eviction + consolidation:
isImmutablebranches deleted from normalizrObject.tsand endpointObject.ts/Polymorphic.ts; endpoint'sImmutableUtils.tsdeleted entirely (endpoint main now has zero immutable code — no/immsubpath needed). Single consolidatedImmutableUtils.tsin normalizr, imported only from/immmodules; the symbol-semantics drift resolved on propagating the first symbol (preservesINVALIDidentity across package boundaries), with tests.Loud failure instead of silent corruption: immutable input to main-entry denormalize now throws a descriptive error in development (prod-stripped), replacing silently corrupt spread output. Former cell-③ tests converted to rejection assertions.
Verification
8-*inexamples/benchmark/micro.js), baselined pre-change: object-shorthand denormalize +12% (8,020 → ~9,000 ops/sec), object-class +5%, union-polymorphic +2%.denormalizeShort donotcache−1.4% anddenormalizeLong Valuesat parity (≤2% gate);core ^getwithin noise.__ownerIDmarker gone from production bundles, now enforced by a newimmutable-leakCI job inbundle_size.yml./immargs fix) + minor/breaking (main-entry immutable-input drop with migration guide). README +docs/api.mdupdated; new exportsPlainValuePolicy,ImmValuePolicy,IValuePolicy.Open questions
INVALID(normalizr's previous behavior). Observable behavior is unchanged in practice since the only symbol in flight is normalizr'sINVALID, but flagging the deliberate resolution for review.