Repository navigation
fix: harden apply() patch paths and fix mark copies, current() snapshots and async recipe types - #192
Merged
Merged
Conversation
|
Coverage after merging fix/v2-review-r5-r11 into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…nd results of apply()
unadlib
force-pushed
the
fix/v2-review-r5-r11
branch
from
October 8, 2026 11:53
209c5c2 to
4b9ec16
Compare
|
Coverage after merging fix/v2-review-r5-r11 into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
unadlib
changed the title
unadlib
force-pushed
the
fix/v2-review-r5-r11
branch
from
October 8, 2026 12:54
4b9ec16 to
fa66997
Compare
|
Coverage after merging fix/v2-review-r5-r11 into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Part of #168.
Summary
This PR handles eight issues that npm 1.3.0 already had. Four get a runtime fix, one a type fix, and three whose fixes cost more than their rare cases justify are documented as limits:
mainand 1.3.0apply()withmutable: trueand a path that ends in__proto__mutablemode; the default mode throws__proto__earlier in a path, in every mode, also for a key that converts to__proto__markSimpleObjector animmutablemark, and an own__proto__key asJSON.parse()creates one__proto__property, as the default copy already didapply()with a Map key that is an object without a prototypeTypeError: Cannot convert object to primitive value; other object keys run theirSymbol.toPrimitiveTypeErrorunion(),intersection()and the other ES2025 Set methods on a Set draft of objects, with original objectshas()accepts the original objectscurrent(draft),original(draft)or idscreate()andapply()create<State>(base, async (draft) => …)is typedStatebut returns a Promise;apply()rejectsenableAutoFreeze: trueand types amutableoption of typebooleanasStatePromise<State>;Immutable<State>;State | voidcurrent()of an object that the recipe assigned and that a mark makes draftablefinalize()that throwsfinalize()returns a revoked draftvmcontextChanges, one commit per item
Commits 1 to 8 handle the issues of the table in its order; 9 to 11 follow up on them.
fix: reject patches that end in __proto__ on objects and arrays:apply()checked__proto__andconstructoronly before the last segment of a path; the last segment is assigned, and an assignment to__proto__sets the prototype. It now throws error 13 there too, for objects and arrays; a Map key named__proto__stays an ordinary key. Withoutmutable, the patch failed before as well, with error 3.fix: keep an own __proto__ key as a property in copies for marks:strictCopy(), which copies objects for marks, assigned plain properties; an own__proto__property now goes throughdefineProperty, as the other copies already do.fix: use Map keys of patch paths as they are in apply():apply()converted every key of a path to a string before checking it, which is needed for objects and arrays only.docs: describe how the Set methods compare the objects of a Set draft: a README and website FAQ entry. These methods compare elements as iterating the draft returns them, drafts for objects of the base state; making them agree withhas()while keeping drafts in their results needs a design and about 60–100 B, for a rare case that Immer gets wrong as well.fix: type async recipes with an explicit state type and the options and results of apply(): with an explicit state type, TypeScript infers no other type parameter, so the return type of the recipe took its default,void, which accepts any function. Four overloads that apply only when the call supplies the state type now precede the existing ones of each form, a synchronous and an async one for direct and for curried calls; they tell an async recipe by its return type. TypeScript cannot infer their state type from the base or from the result, so calls without a type argument resolve to the existing overloads as before. A Promise type that no actual Promise matches gives the synchronous overload the context to keep the literal values that an async recipe returns.apply()acceptsenableAutoFreeze: trueand returnsImmutable<State>with it, asapply<State, true>()does now; amutableorenableAutoFreezeoption of typeboolean, or an optional one, givesState | voidorState | Immutable<State>.test/types.test.tschecks these types withexpectTypeOf, and the packed consumer check compiles typical calls, also withstrictNullChecksdisabled.fix: snapshot assigned objects that a mark makes draftable in current():current()passes the options of the draft above to the objects that the recipe assigned, so a mark makes them draftable there too.docs: say that a manual finalize() that throws leaves its drafts writable: README and the websitecreate()and currying docs. You chose to document this limit when it came up for fix: reject Map keys that are not strings and symbol keys in string patch paths in development builds #189; revoking the drafts would cost 13 B in the production CJS bundle and 35 B in an ESM bundle ofcreate.docs: say how to use an async recipe from another realm: README and the websitecreate()docs, with a wrapper that returns the result of the recipe, so a replacement passes through. Immer has the same limit; recognizing Promises of other realms would cost 11 B.docs: add the note on regrown array lengths to the README migration guide: fix: keep inserted base elements as they are and patch regrown indices of array drafts #190 added this bullet to the website migration guide only; both guides now have the same 38 bullets.fix: normalize property keys before checking patch paths:apply()compared the keys of a path with__proto__,constructorandprototypebefore JavaScript converted them for the property access, so a key such as a boxed string or the array['__proto__']passed the check and then named the reserved key. Object and function keys are now converted once, as a property access converts them, and the check uses the result; a key whose conversion returns a symbol stays a symbol. Arrayaddandremovepatches keep their splice indices.test: compile the packed consumer with TypeScript 5.0: overload resolution and contextual typing differ between TypeScript versions, so the packed consumer check also compiles with TypeScript 5.0.4, a new development dependencytypescript-5.0. CI otherwise checks the types with TypeScript 5.8 only.The fixes add their changes to the "Fixes that change results" section of the migration guide, and commit 5 adds a TypeScript section.
Behavior changes to review
__proto__on an object or array throws error 13 in every mode; withoutmutableit threw error 3 before. A path key that converts to a reserved name, such as a boxed string, is checked after the conversion.Promise<State>, and so does a curried producer. Synchronous recipes keep their result types, calls without a type argument resolve as before, and type errors stay where they were.apply<State, true>()now returnsImmutable<State>instead ofState, and widened or optionalmutableandenableAutoFreezeoptions give the union results above.Not covered
objectorunknown, and generic wrappers that pass their own type parameter, ascreate<T>(state, recipe)does, still give the synchronous type for async recipes, and a replacement that does not match the state type is not reported. Also not covered: curried producers with an annotated draft that return a new state, the types of Map keys in string patch paths, andcreate(1)without a recipe, which the types accept and the runtime rejects.Size
size-limitmeasures 8,387 B for the production CJS bundle (8,367 B onmain; the cap is 8,400 B), 7,341 B for an ESM bundle ofcreate(unchanged) and 8,236 B for all ESM exports (8,225 B). The production CJS artifact shrinks by 22 B raw and grows by 17 B Brotli; in thesize-limitfigure, the key normalization of commit 10 saves 10 B. All artifacts stay within the tolerance of the size baseline, so the baseline is not refreshed. The declaration ofcreategrows from 1,823 B to 4,795 B.Performance
The fixes leave the draft read and write paths alone: items 1 and 10 add comparisons per path segment in
apply(), item 3 removes string conversions from it, item 6 passes the options incurrent(), and item 2 adds one comparison per property when a mark copies an object. In isolated processes,mainand this PR alternating over three rounds at 100 rows,apply-update-10pct,apply-array-opsandapply-reversewith and without auto-freeze have a geometric mean of 0.997 over 6 cells, each between 0.99 and 1.01.class-updateandclass-wide-update, which do not runapply(), measured between 0.99 and 1.02 before.Type-checking against the published declarations takes as many instantiations as on
mainfor a direct call without a type argument, 12 more for a curried one, and 127 instead of 83 for a call with an explicit state type; the time per call differs frommainby at most 0.4 ms, also for a state of 40 nested fields.Verification
TypeError, and item 10 in 4 tests; on the types ofmain,test/types.test.tshas 44 type errors.test/types.test.tspasses against the built declarations with TypeScript 5.0.4, 5.4.5, 5.6.3, 5.7.3, 5.9.3, 6.0.3 and 7.0.2, and typical calls resolve the same on TypeScript 4.8.4 to 7.0.2.srcstays at 100% of lines, branches and functions, and each commit passes the tests,type-check, lint and the format check.