Repository navigation
fix: harden apply() for Set positions and mutable root replacement, and fix development checks and recipe types - #201
Merged
Merged
Conversation
… when freezing in development
|
Coverage after merging fix/v2-correctness-follow-ups into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…imitive state type
…nt check of draft keys
|
Coverage after merging fix/v2-correctness-follow-ups into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
… instances assign
|
Coverage after merging fix/v2-correctness-follow-ups into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging fix/v2-correctness-follow-ups into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This was referenced Oct 11, 2026
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
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 ten issues in patches, development checks, types and documentation, all of which npm 1.3.0 already had, and two release tasks. Six issues get a fix, three whose fixes cost more than their rare cases justify are documented as limits, and one claim of the README is corrected.
mainand 1.3.0apply()with a patch path that passes a Set__proto__reachesArray.prototypeand the patch changes it1or'1'; any other key, such as__proto__, throwsapply()withmutable: trueand a patch that replaces the root statemutablemodeenableAutoFreezein development builds, when a changed draft holds its original, asdraft.prev = original(draft)makes it, or links to a shared object through another pathForbids circular reference, while production builds freeze the stateset()of a Map draftcreate()throws aTypeErrorTypeErrorcreate((draft: State) => ({ ...draft, count: 0 }))State, orPromise<State>for an async recipe(draft: State) => void | Promise<void>State, although the result can be a PromiseState | Promise<State>for state types other than primitives, also for curried producers and creators frommakeCreator()apply()appends the elements that it adds, so the Set it changes can hold another order, and the positions in later patches find other elementscreate()call stored in the draft of anothersetUseStrictShallowCopy('class_only')defines the propertiesChanges, one commit per item
fix: report a circular reference only for an object that holds itself when freezing in development(item 3): development builds mapped the copy of each changed draft to its original before looking for the object among its ancestors, so a copy that holds its original, or a shared object that the recipe linked through another path, counted as a cycle. They now compare the objects of the state as they are. A real cycle still throws, and its error names the path of the object that repeats, one key longer in 6 existing snapshots. Without the mapping, the bookkeeping that recorded the originals during finalization is gone, also from the production builds.fix: find a Set item of a patch path only at a numeric position(item 1):apply()exempted Sets from the check of reserved keys, since their path segments are positions, and read the segment as a property ofArray.from(set). The segment was converted to a number first, so that__proto__and other names found nothing and threw error 14; commit 17 replaces the conversion with a check for an index. Map keys keep their native semantics.fix: throw when a mutable apply() would replace the root state(item 2): a mutable application changes the state in place and returns nothing, so it cannot apply a root replacement, which the patches of a recipe that returns a new state contain. It now throws error 21 before changing anything, unless the replacement is the state itself, as in the patch of a recipe that returns its base state. Withoutmutable, root replacements work as before. mutativejs/travels, which applies patches in place, already falls back to an immutable update for such patches. The errors page, theapply()docs and the Patches guide describe the error.fix: throw in development builds when a draft becomes a Map key(item 4): Map keys are not drafted and are used as they are, so a Map draft that receives a draft as a key keeps a key that is revoked when the producer ends. Development builds now throw inset()and nameoriginal(key),current(key)or an id as the alternatives; production builds keep the key, as before. A FAQ entry in the README and on the website explains it.fix: type curried producers whose recipe annotates its draft and returns a new state(item 5): the overload for inferred curried producers constrained the return type of the recipe tovoid | Promise<void>, so a recipe that returns a new state fell through to the manual draft overload. It now also acceptsTandPromise<T>, as the overload for direct calls already did.fix: type the results of explicit-state recipes that may return a Promise(item 6): the explicit-state overloads from fix: harden apply() patch paths and fix mark copies, current() snapshots and async recipe types #192 tell synchronous and async recipes apart, and a recipe that can return either matched neither, so it fell through to an overload whose recipe return type defaults tovoid, which accepts any function. One more overload for direct calls and one for curried calls giveResult | Promise<Result>; commit 13 keeps them away from primitive state types.docs: state that a Set that apply() changes can hold its elements in another order(item 7): the Patches guide, the READMEapply()note and the comparison with Immer.docs: keep drafts of one create() call out of the drafts of another(item 8): the README and the websitecreate()page.docs: describe how Mutative copies instances that a mark makes draftable(item 9): a section in the mark guide, themarkoption in the README and on thecreate()page, and the comparison with Immer, which said that the copy works likeclass_only.docs: describe the JSON Patch support of patches precisely(item 10).ci: type-check the source and tests before publishing: the publish workflow built, checked and tested the package but did not runpnpm type-check, which also checkstest/types.test.ts.docs: regenerate the API reference: the typedoc output indocs/dated from November 2024;createandmakeCreatornow appear as variables.fix: keep the result types of synchronous recipes with an explicit primitive state type(item 6): the overloads of commit 6 also took the synchronous recipes of a primitive state, which skip the first explicit overload because it only gives async recipes their context, socreate<number>(0, (n) => n + 1)becamenumber | Promise<number>, also with patches, in curried producers and in creators frommakeCreator(). These overloads no longer apply to primitive state types, which keep the result types ofmain.fix: accept Map keys whose properties cannot be read in the development check of draft keys(item 4): the check of commit 4 read the symbol of drafts on every key, so a Proxy that rejects reading properties, or a revoked one, threw wheremainaccepted it. A key whose symbol cannot be read is no draft.docs: show the signatures of makeCreator() in the API reference: the page that commit 12 generated named only the type ofmakeCreator; an@inlinetag on that type lets typedoc show its type parameters, its options and the signatures of the creator it returns.build: refresh the size baseline for the development checks of Map keys and cycles: the development builds shrink by 445–462 B raw, but their new error messages grow them by 19–69 B compressed, over the allowance of 64 B for two of them.fix: find a Set item of a patch path only at its index(item 1): the conversion of commit 2 also found items under keys that are no index, which threw onmain:'',' ','-0',null,falseand[]found the first item, andtrue,'01','1.0','1e0','0x1'and'+1'the second, so the string path/set//vchanged the first item. A key that is not a number must now be an own index of the array of the Set's items, soapply()accepts the keys thatmainaccepts there, apart from inherited properties such as__proto__, and throws for the others, as before. Numbers skip the check, as no number names an inherited property: checking them too made a mutable application of 10 patches through a Set of 10 items 6% slower than onmain.docs: name all attributes of the own properties that copies of marked instances assign(item 9): the notes of commit 9 in the README, on thecreate()page and in the comparison with Immer said that the enumerable and writable own properties are assigned; they must also be configurable, so the properties of a sealed instance are defined, and no setter runs.docs: state that reading the properties of a draft kept as a Map key throws(item 4): the FAQ entry and the migration note of commit 4 said that such a key throws when it is read, while only reading its properties throws.build: refresh the size baseline for the index check of Set items in patch paths: commit 17 grows the development builds by 88–92 B raw over the baseline of commit 16, beyond its allowance of 64 B.fix: convert the key of a Set step in a patch path once(item 1): the check of commit 17 and the read after it converted an object key separately, so a key whoseSymbol.toPrimitivereturns another value on each call passed the check as'0'and was read as'__proto__', which let the patch changeArray.prototype, or as'1', which changed another item. The Set step now converts the key once, as the steps through objects and arrays do, and checks and reads the converted key; Map keys keep their identity. Such a key is a JavaScript object, which JSON patches cannot hold.Commits 1 to 6 add their changes to the migration guide in the README and on the website; commits 13, 17 and 19 adjust those of commits 6, 2 and 4.
Behavior changes to review
1or'1'; inherited properties of the array of the Set's items, such as__proto__, now throw error 14, and other keys throw as onmain.apply()withmutable: truethrows error 21 for a root replacement instead of ignoring it.main.Not covered
The fixes for these limits cost more than their rare cases justify; each was measured:
apply()would then copy the whole Set, 3–44x slower, and the patch for one new element of a Set of 10,000 numbers would grow from 44 B to 49 KB of JSON. Inserting added elements at their positions instead, about 35–50 B, keeps the order but leavesremovepatches matching objects by identity, so Sets thatapply()built would still fail.class-wide-updateat 1,000 fields with auto-freeze: 497 µs instead of 416 µs onmain); caching that check per prototype costs about 90 B, over the size caps; defining every property, asclass_onlydoes, is 2–5.5x slower.main: an overload that takes only synchronous recipes would fix it, at more type-checking cost for every curried producer.options?: { enablePatches: true }, still gives the tuple type when the options are missing; telling these calls apart needs other overloads for every form.Two older gaps, found in review, are left for later;
mainhas both:Symbol.unscopablesreaches the object thatArray.prototypeholds under it, and the patch changes that object, with or withoutmutable. JSON has no symbols, so patches parsed from JSON cannot do this.Promise<State | undefined>, does not compile in a direct call without a state type, and a curried producer whose recipe annotates its draft is typed as a manual draft; with an explicit state type, both givePromise<State>.Size
The production CJS artifact grows from 27,353 B to 27,375 B raw and from 8,439 B to 8,449 B Brotli; the UMD and ESM production artifacts shrink by 2 B Brotli.
size-limitmeasures 8,615 B for it (8,612 B onmain), 7,546 B for an ESM bundle ofcreate(7,554 B) and 8,485 B for all ESM exports (8,477 B), within the caps of 8.7, 7.7 and 8.5 kB. Consumer bundles built with esbuild change by −6 to +25 B Brotli. The development builds shrink by 322–339 B raw and grow by 33–88 B Brotli, so commits 16 and 20 refresh the size baseline. Commit 17 adds 23 B Brotli to the production CJS artifact, 8 B of them for skipping the check of numbers, and commit 21 adds 3 B; commits 13 to 16 and 18 to 20 leave the production artifacts byte-identical. The declaration ofmakeCreatorgrows from 3,257 B to 4,064 B with the overloads of commits 6 and 13.Performance
The paired performance budgets of CI on the head compared 146 latency cells in five groups: geometric mean 0.998, the slowest cell 1.059 (
return-replacewith patches and without freezing, whose code this PR does not change), and theapply-*cells 0.987–1.005. On earlier heads they gave 0.996, 1.036 and 0.985–1.000 (commit 12), 1.002, 1.076 and 0.991–1.017 (commit 16, with the production artifacts of commit 12), and 0.997, 1.084 and 0.986–1.012 (commit 20). In one process, alternating the production CJS builds ofmainand this PR over 16 rounds: updating 10% of 1,000 rows with auto-freeze 1.008, assigning 1,000 new rows with auto-freeze 0.988, applying 100 patches through a Set 0.968–1.023 over four runs, a mutable application of 100 patches through a Set 1.005 and of 10 patches through a Set of 10 items 1.008, and a mutable application of 100 patches 0.999.In development builds,
set()on a Map draft takes 9% longer with 1,000 object keys for the check of commits 4 and 14, and 3% longer with string keys, while producers with auto-freeze run 10–14% faster without the bookkeeping that commit 1 removes.Type-checking against the built declarations takes at most 8 more instantiations than on
mainfor 150 direct calls, with or without a state type, or 150 curried calls with a state type. Inferred curried producers with an annotated draft take about 18% more (7,613 instead of 6,431 for 150 calls), and 750 calls of five shapes take 11,153 instantiations instead of 9,971 and check in 0.26 s instead of 0.24 s.Verification
main, item 1 in 1 test, item 2 in 2 (one of them for the production error code), item 3 in 2 and item 4 in 4, the test of commit 14 fails with the check of commit 4, the test of commit 17 with the conversion of commit 2, and the test of commit 21 with the check of commit 17; without items 5 and 6,test/types.test.tsreports 6 and 4 type errors, and without commit 13, 8 for the result types of primitive states.mainwith TypeScript 4.8.4 to 7.0.2, apart from the calls that items 5 and 6 fix; the calls include explicit primitive and literal state types with synchronous, void, async and curried recipes and creators frommakeCreator(), which also resolve as onmainwithstrictNullChecksoff. TypeScript 4.7.4 crashes on both, as before.type-check, the build,pnpm size,test:package,test:build-watchandtest:benchmarks(35 tests); coverage ofsrcstays at 100% of statements, branches, functions and lines. Each commit passes the format check, lint,type-checkand the tests on its own.