From a003b188e983fc8d37440d6582c946b3537cf176 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:07:51 +0200 Subject: [PATCH 1/6] chore(deps): bump changesets/action from 1.9.0 to 2.1.0 (#4691) Bumps [changesets/action](https://github.com/changesets/action) from 1.9.0 to 2.1.0. - [Release notes](https://github.com/changesets/action/releases) - [Changelog](https://github.com/changesets/action/blob/main/CHANGELOG.md) - [Commits](https://github.com/changesets/action/compare/v1.9.0...v2.1.0) --- updated-dependencies: - dependency-name: changesets/action dependency-version: 2.1.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index aa4a16087..338865b04 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -40,7 +40,7 @@ jobs: - name: Create Release Pull Request or Publish to npm id: changesets - uses: changesets/action@v1.9.0 + uses: changesets/action@v2.1.0 with: # This expects you to have a script called release which does a build for your packages and calls changeset publish publish: npm run release From 043850ed96266f8bea42bf643a65e62245a98b3c Mon Sep 17 00:00:00 2001 From: Gianluca Esposito Date: Wed, 19 Aug 2026 10:12:00 +0200 Subject: [PATCH 2/6] fix: Fix memory leak in useObserver [keeping a reference to first rendered component tree and its transitive closures] (#4689) subscribe/getSnapshot were created inside useObserver's first invocation and shared that invocation's closure context with the reaction.track callback's captures (render, renderResult). Since useSyncExternalStore holds subscribe for as long as the component is mounted, the element tree returned by the first render (and every fiber and DOM node reachable from it) could never be garbage collected. Create the administration object in a module-level factory instead, so subscribe/getSnapshot close over nothing render-related. --- .../use-observer-first-render-retention.md | 5 ++ .../__tests__/useObserverRetention.test.tsx | 50 ++++++++++++ packages/mobx-react-lite/src/useObserver.ts | 79 +++++++++++-------- 3 files changed, 99 insertions(+), 35 deletions(-) create mode 100644 .changeset/use-observer-first-render-retention.md create mode 100644 packages/mobx-react-lite/__tests__/useObserverRetention.test.tsx diff --git a/.changeset/use-observer-first-render-retention.md b/.changeset/use-observer-first-render-retention.md new file mode 100644 index 000000000..b274e9a4f --- /dev/null +++ b/.changeset/use-observer-first-render-retention.md @@ -0,0 +1,5 @@ +--- +"mobx-react-lite": patch +--- + +fix: `useObserver` no longer retains the element tree returned by a component's first render for the component's whole mounted life. `subscribe`/`getSnapshot` were created inside `useObserver`'s first invocation and therefore shared that invocation's closure context with the `reaction.track` callback's captures (`render`, `renderResult`); since React's `useSyncExternalStore` holds `subscribe` while the component is mounted, the first render result (and every fiber and DOM node reachable from it) could never be garbage collected. The administration object is now created by a module-level factory whose scope contains nothing render-related. diff --git a/packages/mobx-react-lite/__tests__/useObserverRetention.test.tsx b/packages/mobx-react-lite/__tests__/useObserverRetention.test.tsx new file mode 100644 index 000000000..4dba59eae --- /dev/null +++ b/packages/mobx-react-lite/__tests__/useObserverRetention.test.tsx @@ -0,0 +1,50 @@ +import { render } from "@testing-library/react" +import * as React from "react" +import gc from "expose-gc/function" +import { observer } from "../src" + +function nextFrame() { + return new Promise(accept => setTimeout(accept, 1)) +} + +async function gc_cycle() { + await nextFrame() + gc() + await nextFrame() +} + +// If `subscribe` shares a closure context with the `reaction.track` callback, +// the first render's result stays reachable for as long as the component is mounted. +// See the comment on `createObserverAdministration` in src/useObserver.ts. +test("a mounted observer does not retain the element tree it first returned", async () => { + let capturedFirstTree: object | null = null + + const TestComponent = observer(function TestComponent({ label }: { label: string }) { + const tree =
+ if (!capturedFirstTree) { + capturedFirstTree = tree + } + return tree + }) + + const rendering = render() + + const weakFirstTree = new WeakRef(capturedFirstTree!) + capturedFirstTree = null + + // Several re-renders rather than one: React double-buffers fibers, so the + // `alternate` fiber legitimately keeps the immediately-previous render's + // tree alive for one extra commit. Distinct props are required because + // `observer` wraps the component in `React.memo`. + rendering.rerender() + rendering.rerender() + rendering.rerender() + + await gc_cycle() + + expect(weakFirstTree.deref()).toBeUndefined() +}) + +// There is deliberately no assertion that the first render's `props` become +// collectable: a control component wrapped in plain `React.memo` (no MobX involved) +// fails such an assertion too, so it cannot isolate MobX's contribution. diff --git a/packages/mobx-react-lite/src/useObserver.ts b/packages/mobx-react-lite/src/useObserver.ts index 08da5b53f..2c26a9cc3 100644 --- a/packages/mobx-react-lite/src/useObserver.ts +++ b/packages/mobx-react-lite/src/useObserver.ts @@ -30,6 +30,49 @@ function createReaction(adm: ObserverAdministration) { }) } +// This must stay a module-level factory and must NOT be inlined into `useObserver`: +// all closures created during the same function invocation share one context object. +// If `subscribe`/`getSnapshot` were created inside `useObserver`, that shared context +// would also hold the `reaction.track` callback's captures (`render`, `renderResult`), +// and since `useSyncExternalStore` holds `subscribe` for as long as the component is +// mounted, every observer would retain its first render's element tree, fibers and DOM. +// Module scope also makes it impossible for these closures to capture `admRef`, +// which would prevent its collection and break leaked-reaction disposal via the +// FinalizationRegistry (see the comment on `ObserverAdministration`). +function createObserverAdministration(baseComponentName: string): ObserverAdministration { + const adm: ObserverAdministration = { + reaction: null, + onStoreChange: null, + stateVersion: Symbol(), + name: baseComponentName, + subscribe(onStoreChange: () => void) { + observerFinalizationRegistry.unregister(adm) + adm.onStoreChange = onStoreChange + if (!adm.reaction) { + // We've lost our reaction and therefore all subscriptions, occurs when: + // 1. Timer based finalization registry disposed reaction before component mounted. + // 2. React "re-mounts" same component without calling render in between (typically ). + // We have to recreate reaction and schedule re-render to recreate subscriptions, + // even if state did not change. + createReaction(adm) + // `onStoreChange` won't force update if subsequent `getSnapshot` returns same value. + // So we make sure that is not the case + adm.stateVersion = Symbol() + } + + return () => { + adm.onStoreChange = null + adm.reaction?.dispose() + adm.reaction = null + } + }, + getSnapshot() { + return adm.stateVersion + } + } + return adm +} + export function useObserver(render: () => T, baseComponentName: string = "observed"): T { if (isUsingStaticRendering()) { return render() @@ -39,41 +82,7 @@ export function useObserver(render: () => T, baseComponentName: string = "obs if (!admRef.current) { // First render - const adm: ObserverAdministration = { - reaction: null, - onStoreChange: null, - stateVersion: Symbol(), - name: baseComponentName, - subscribe(onStoreChange: () => void) { - // Do NOT access admRef here! - observerFinalizationRegistry.unregister(adm) - adm.onStoreChange = onStoreChange - if (!adm.reaction) { - // We've lost our reaction and therefore all subscriptions, occurs when: - // 1. Timer based finalization registry disposed reaction before component mounted. - // 2. React "re-mounts" same component without calling render in between (typically ). - // We have to recreate reaction and schedule re-render to recreate subscriptions, - // even if state did not change. - createReaction(adm) - // `onStoreChange` won't force update if subsequent `getSnapshot` returns same value. - // So we make sure that is not the case - adm.stateVersion = Symbol() - } - - return () => { - // Do NOT access admRef here! - adm.onStoreChange = null - adm.reaction?.dispose() - adm.reaction = null - } - }, - getSnapshot() { - // Do NOT access admRef here! - return adm.stateVersion - } - } - - admRef.current = adm + admRef.current = createObserverAdministration(baseComponentName) } const adm = admRef.current! From 030498d6b2bfe4cc27340fed8c706177cb3b28e1 Mon Sep 17 00:00:00 2001 From: Abdelrahman Youssef <49205789+a-y-ibrahim@users.noreply.github.com> Date: Wed, 19 Aug 2026 11:14:41 +0300 Subject: [PATCH 3/6] fix: Fix stack overflow in endBatch() when disposing a Reaction from onBecomeUnobserved (#4684) * Fix stack overflow in endBatch() when disposing a Reaction from onBecomeUnobserved endBatch()'s pendingUnobservations drain loop calls onBUO() per observable, which can dispose a Reaction (a common pattern). Reaction.dispose() calls startBatch()/endBatch() again, and since inBatch is already back at 0 at that point, the nested endBatch() recursed into the same drain loop instead of returning, unlike runReactions() which already guards against this exact kind of reentrancy with isRunningReactions. For long enough chains of onBecomeUnobserved handlers disposing reactions, this recursion overflowed the stack (issue #3954). Add an analogous isRunningUnobservations guard: a nested endBatch() call now just returns, and the already-running outer loop picks up anything a nested dispose() pushes onto pendingUnobservations because it re-reads list.length on every iteration. * Address review feedback: release isRunningUnobservations via try/finally If an onBecomeUnobserved handler threw, the guard flag never got reset, permanently disabling pendingUnobservations draining for the rest of the process. Wrap the drain in try/finally so it releases regardless of how the block exits; pendingUnobservations itself is only cleared on the success path, unchanged from before this fix, so a thrown exception still leaves the array for a later pass to pick back up. Added a regression test: a handler that throws still surfaces the error, and a second, unrelated disposal right after still fires its own handler instead of silently no-oping forever. --- .../fix-endbatch-unobservation-reentrancy.md | 5 ++ .../mobx/__tests__/base/become-observed.ts | 52 +++++++++++++++++++ packages/mobx/src/core/globalstate.ts | 8 +++ packages/mobx/src/core/observable.ts | 45 ++++++++++------ 4 files changed, 95 insertions(+), 15 deletions(-) create mode 100644 .changeset/fix-endbatch-unobservation-reentrancy.md diff --git a/.changeset/fix-endbatch-unobservation-reentrancy.md b/.changeset/fix-endbatch-unobservation-reentrancy.md new file mode 100644 index 000000000..9ebd53c29 --- /dev/null +++ b/.changeset/fix-endbatch-unobservation-reentrancy.md @@ -0,0 +1,5 @@ +--- +"mobx": patch +--- + +Fix a stack overflow ("Maximum call stack size exceeded") that could occur when an `onBecomeUnobserved` handler disposes a `Reaction`. Disposing a `Reaction` re-enters `endBatch()`, which used to recurse into the same `pendingUnobservations` drain loop instead of letting the already-running outer loop pick up the newly queued items, causing unbounded stack depth for long enough chains. diff --git a/packages/mobx/__tests__/base/become-observed.ts b/packages/mobx/__tests__/base/become-observed.ts index a8fd6d674..ed73345fd 100644 --- a/packages/mobx/__tests__/base/become-observed.ts +++ b/packages/mobx/__tests__/base/become-observed.ts @@ -502,6 +502,58 @@ test("#2667", () => { ]) }) +test("#3954 - disposing a chain of reactions from onBecomeUnobserved doesn't overflow the stack", () => { + // Each box's onBecomeUnobserved handler disposes the next reaction in the + // chain. Disposing reaction[0] unobserves box[0], whose handler disposes + // reaction[1], which unobserves box[1], and so on. Each of those disposals + // re-enters endBatch() while the previous one is still draining + // pendingUnobservations, so this used to recurse N deep instead of + // looping, overflowing the stack for a large enough chain. + const N = 10000 + const boxes = Array.from({ length: N }, () => observable.box(0)) + const disposers = boxes.map(box => autorun(() => box.get())) + let unobservedCount = 0 + + boxes.forEach((box, i) => { + onBecomeUnobserved(box, () => { + unobservedCount++ + if (i + 1 < N) { + disposers[i + 1]() + } + }) + }) + + expect(() => disposers[0]()).not.toThrow() + + // the whole chain should have unwound, not just the first link + expect(unobservedCount).toBe(N) +}) + +test("#3954 followup - isRunningUnobservations is released even if an onBecomeUnobserved handler throws", () => { + const boxA = observable.box(0) + const disposeA = autorun(() => boxA.get()) + onBecomeUnobserved(boxA, () => { + throw new Error("boom") + }) + + // the handler's exception should still surface to the caller, not be swallowed + expect(() => disposeA()).toThrow("boom") + + // if the internal guard were left stuck true after that exception, every + // future endBatch() would silently stop draining pendingUnobservations, + // so this completely unrelated disposal would never fire its own handler + const boxB = observable.box(0) + const disposeB = autorun(() => boxB.get()) + let unobservedB = false + onBecomeUnobserved(boxB, () => { + unobservedB = true + }) + + disposeB() + + expect(unobservedB).toBe(true) +}) + test("works with ObservableSet #3595", () => { const onSetObserved = jest.fn() const onSetUnobserved = jest.fn() diff --git a/packages/mobx/src/core/globalstate.ts b/packages/mobx/src/core/globalstate.ts index a1c4de647..d06a9a7ce 100644 --- a/packages/mobx/src/core/globalstate.ts +++ b/packages/mobx/src/core/globalstate.ts @@ -82,6 +82,14 @@ export class MobXGlobals { */ isRunningReactions = false + /** + * Are we currently draining pendingUnobservations in endBatch? + * An onBecomeUnobserved handler can dispose a Reaction, which calls + * startBatch/endBatch again; this guards against re-entering the same + * drain loop recursively (see endBatch in observable.ts). + */ + isRunningUnobservations = false + /** * Is it allowed to change observables at this point? * In general, MobX doesn't allow that when running computations and React.render. diff --git a/packages/mobx/src/core/observable.ts b/packages/mobx/src/core/observable.ts index b79c63939..af4b75a2e 100644 --- a/packages/mobx/src/core/observable.ts +++ b/packages/mobx/src/core/observable.ts @@ -108,24 +108,39 @@ export function endBatch() { if (--globalState.inBatch === 0) { runReactions() // the batch is actually about to finish, all unobserving should happen here. - const list = globalState.pendingUnobservations - for (let i = 0; i < list.length; i++) { - const observable = list[i] - observable.isPendingUnobservation = false - if (observable.observers_.size === 0) { - if (observable.isBeingObserved) { - // if this observable had reactive observers, trigger the hooks - observable.isBeingObserved = false - observable.onBUO() - } - if (observable instanceof ComputedValue) { - // computed values are automatically teared down when the last observer leaves - // this process happens recursively, this computed might be the last observabe of another, etc.. - observable.suspend_() + // Guard against re-entering this loop: an onBUO handler can dispose a Reaction, + // which calls startBatch/endBatch again while we're still iterating. Bail out of + // the nested call instead of recursing; the outer loop re-reads list.length on + // every iteration, so it picks up anything the nested dispose() pushes onto the + // same pendingUnobservations array. + if (!globalState.isRunningUnobservations) { + globalState.isRunningUnobservations = true + try { + const list = globalState.pendingUnobservations + for (let i = 0; i < list.length; i++) { + const observable = list[i] + observable.isPendingUnobservation = false + if (observable.observers_.size === 0) { + if (observable.isBeingObserved) { + // if this observable had reactive observers, trigger the hooks + observable.isBeingObserved = false + observable.onBUO() + } + if (observable instanceof ComputedValue) { + // computed values are automatically teared down when the last observer leaves + // this process happens recursively, this computed might be the last observabe of another, etc.. + observable.suspend_() + } + } } + globalState.pendingUnobservations = [] + } finally { + // Always release the guard, even if an onBUO handler (user code) threw, + // otherwise every future endBatch() would see isRunningUnobservations + // stuck true and silently stop draining pendingUnobservations forever. + globalState.isRunningUnobservations = false } } - globalState.pendingUnobservations = [] } } From 0d003d17f8497f9a15aef2cd9eca091f89d70a4a Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 19 Aug 2026 10:52:41 +0200 Subject: [PATCH 4/6] chore: Revert "chore(deps): bump changesets/action from 1.9.0 to 2.1.0 (#4691)" (#4693) This reverts commit a003b188e983fc8d37440d6582c946b3537cf176. --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 338865b04..aa4a16087 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -40,7 +40,7 @@ jobs: - name: Create Release Pull Request or Publish to npm id: changesets - uses: changesets/action@v2.1.0 + uses: changesets/action@v1.9.0 with: # This expects you to have a script called release which does a build for your packages and calls changeset publish publish: npm run release From c65a4e14cf48b42cb792dc4c64edbcf56234b32d Mon Sep 17 00:00:00 2001 From: Gianluca Esposito Date: Wed, 19 Aug 2026 10:53:39 +0200 Subject: [PATCH 5/6] fix(perf): lazily allocate observers_ Set to reduce memory for unobserved atoms (#4682) Co-authored-by: Michel Weststrate --- .changeset/lazy-observers-allocation.md | 5 ++++ packages/mobx/__tests__/base/errorhandling.js | 4 +-- packages/mobx/__tests__/base/observables.js | 6 ++-- packages/mobx/src/core/atom.ts | 3 +- packages/mobx/src/core/computedvalue.ts | 5 ++-- packages/mobx/src/core/derivation.ts | 2 +- packages/mobx/src/core/observable.ts | 29 ++++++++++++------- 7 files changed, 34 insertions(+), 20 deletions(-) create mode 100644 .changeset/lazy-observers-allocation.md diff --git a/.changeset/lazy-observers-allocation.md b/.changeset/lazy-observers-allocation.md new file mode 100644 index 000000000..d1d1f48e4 --- /dev/null +++ b/.changeset/lazy-observers-allocation.md @@ -0,0 +1,5 @@ +--- +"mobx": patch +--- + +perf: lazily allocate the internal `observers_` Set. Atoms and computed values no longer allocate an empty `Set` upfront; it is created on first observer instead. Most atoms in large stores are never observed, so this saves roughly 160 bytes per unobserved atom (e.g. ~35% lower heap usage when hydrating 50k instances with 10 observable fields each). diff --git a/packages/mobx/__tests__/base/errorhandling.js b/packages/mobx/__tests__/base/errorhandling.js index 68fd14e2b..f5d36aa9c 100644 --- a/packages/mobx/__tests__/base/errorhandling.js +++ b/packages/mobx/__tests__/base/errorhandling.js @@ -484,7 +484,7 @@ test("peeking inside erroring computed value doesn't bork (global) state", () => }).toThrow(/chocolademelk/) expect(a.isPendingUnobservation).toBe(false) - expect(a.observers_.size).toBe(0) + expect(a.observers_?.size ?? 0).toBe(0) expect(a.diffValue).toBe(0) expect(a.lowestObserverState_).toBe(-1) expect(a.hasUnreportedChange_).toBe(false) @@ -494,7 +494,7 @@ test("peeking inside erroring computed value doesn't bork (global) state", () => expect(b.observing_.length).toBe(0) expect(b.newObserving_).toBe(null) expect(b.isPendingUnobservation).toBe(false) - expect(b.observers_.size).toBe(0) + expect(b.observers_?.size ?? 0).toBe(0) expect(b.diffValue).toBe(0) expect(b.lowestObserverState_).toBe(0) expect(b.unboundDepsCount_).toBe(0) diff --git a/packages/mobx/__tests__/base/observables.js b/packages/mobx/__tests__/base/observables.js index b30ba4f64..cdb43c29f 100644 --- a/packages/mobx/__tests__/base/observables.js +++ b/packages/mobx/__tests__/base/observables.js @@ -1031,7 +1031,7 @@ test("prematurely end autorun", function () { x.get() }) - expect(x.observers_.size).toBe(0) + expect(x.observers_?.size ?? 0).toBe(0) expect(dis1[$mobx].observing_.length).toBe(0) expect(dis2[$mobx].observing_.length).toBe(0) @@ -1282,7 +1282,7 @@ test("prematurely ended autoruns are cleaned up properly", () => { expect(called).toBe(1) expect(a.observers_.size).toBe(1) - expect(b.observers_.size).toBe(0) + expect(b.observers_?.size ?? 0).toBe(0) expect(c.observers_.size).toBe(1) expect(d[$mobx].observing_.length).toBe(2) @@ -1290,7 +1290,7 @@ test("prematurely ended autoruns are cleaned up properly", () => { expect(called).toBe(2) expect(a.observers_.size).toBe(0) - expect(b.observers_.size).toBe(0) + expect(b.observers_?.size ?? 0).toBe(0) expect(c.observers_.size).toBe(0) expect(d[$mobx].observing_.length).toBe(0) }) diff --git a/packages/mobx/src/core/atom.ts b/packages/mobx/src/core/atom.ts index 4000a9124..2b1067d16 100644 --- a/packages/mobx/src/core/atom.ts +++ b/packages/mobx/src/core/atom.ts @@ -30,7 +30,8 @@ const enum AtomFlags { export class Atom implements IAtom { private flags_ = 0b000 - observers_ = new Set() + // Allocated lazily on first observer to save memory. + observers_: Set | null = null lastAccessedBy_ = 0 lowestObserverState_ = IDerivationState_.NOT_TRACKING_ diff --git a/packages/mobx/src/core/computedvalue.ts b/packages/mobx/src/core/computedvalue.ts index d570d7955..3e7433211 100644 --- a/packages/mobx/src/core/computedvalue.ts +++ b/packages/mobx/src/core/computedvalue.ts @@ -84,7 +84,8 @@ export class ComputedValue implements IObservable, IComputedValue, IDeriva dependenciesState_ = IDerivationState_.NOT_TRACKING_ observing_: IObservable[] = [] // nodes we are looking at. Our value depends on these nodes newObserving_ = null // during tracking it's an array with new observed observers - observers_ = new Set() + // Lazily allocated on first observer - see Atom.observers_. + observers_: Set | null = null runId_ = 0 lastAccessedBy_ = 0 lowestObserverState_ = IDerivationState_.UP_TO_DATE_ @@ -200,7 +201,7 @@ export class ComputedValue implements IObservable, IComputedValue, IDeriva if ( globalState.inBatch === 0 && // !globalState.trackingDerivatpion && - this.observers_.size === 0 && + (!this.observers_ || this.observers_.size === 0) && !this.keepAlive_ ) { if (shouldCompute(this)) { diff --git a/packages/mobx/src/core/derivation.ts b/packages/mobx/src/core/derivation.ts index 4ba8c6eb4..323b2688b 100644 --- a/packages/mobx/src/core/derivation.ts +++ b/packages/mobx/src/core/derivation.ts @@ -134,7 +134,7 @@ export function checkIfStateModificationsAreAllowed(atom: IAtom) { if (!__DEV__) { return } - const hasObservers = atom.observers_.size > 0 + const hasObservers = !!atom.observers_ && atom.observers_.size > 0 // Should not be possible to change observed state outside strict mode, except during initialization, see #563 if ( !globalState.allowStateChanges && diff --git a/packages/mobx/src/core/observable.ts b/packages/mobx/src/core/observable.ts index af4b75a2e..3d5351adc 100644 --- a/packages/mobx/src/core/observable.ts +++ b/packages/mobx/src/core/observable.ts @@ -26,7 +26,7 @@ export interface IObservable extends IDepTreeNode { lowestObserverState_: IDerivationState_ // Used to avoid redundant propagations isPendingUnobservation: boolean // Used to push itself to global.pendingUnobservations at most once per batch. - observers_: Set + observers_: Set | null onBUO(): void onBO(): void @@ -36,11 +36,11 @@ export interface IObservable extends IDepTreeNode { } export function hasObservers(observable: IObservable): boolean { - return observable.observers_ && observable.observers_.size > 0 + return !!observable.observers_ && observable.observers_.size > 0 } export function getObservers(observable: IObservable): Set { - return observable.observers_ + return observable.observers_ ?? new Set() } // function invariantObservers(observable: IObservable) { @@ -65,7 +65,7 @@ export function addObserver(observable: IObservable, node: IDerivation) { // invariant(observable._observers.indexOf(node) === -1, "INTERNAL ERROR add already added node"); // invariantObservers(observable); - observable.observers_.add(node) + ;(observable.observers_ ??= new Set()).add(node) if (observable.lowestObserverState_ > node.dependenciesState_) { observable.lowestObserverState_ = node.dependenciesState_ } @@ -78,8 +78,12 @@ export function removeObserver(observable: IObservable, node: IDerivation) { // invariant(globalState.inBatch > 0, "INTERNAL ERROR, remove should be called only inside batch"); // invariant(observable._observers.indexOf(node) !== -1, "INTERNAL ERROR remove already removed node"); // invariantObservers(observable); - observable.observers_.delete(node) - if (observable.observers_.size === 0) { + const observers = observable.observers_ + if (!observers) { + return + } + observers.delete(node) + if (observers.size === 0) { // deleting last observer queueForUnobservation(observable) } @@ -120,7 +124,7 @@ export function endBatch() { for (let i = 0; i < list.length; i++) { const observable = list[i] observable.isPendingUnobservation = false - if (observable.observers_.size === 0) { + if (!observable.observers_ || observable.observers_.size === 0) { if (observable.isBeingObserved) { // if this observable had reactive observers, trigger the hooks observable.isBeingObserved = false @@ -164,7 +168,10 @@ export function reportObserved(observable: IObservable): boolean { } } return observable.isBeingObserved - } else if (observable.observers_.size === 0 && globalState.inBatch > 0) { + } else if ( + (!observable.observers_ || observable.observers_.size === 0) && + globalState.inBatch > 0 + ) { queueForUnobservation(observable) } @@ -202,7 +209,7 @@ export function propagateChanged(observable: IObservable) { observable.lowestObserverState_ = IDerivationState_.STALE_ // Ideally we use for..of here, but the downcompiled version is really slow... - observable.observers_.forEach(d => { + observable.observers_?.forEach(d => { if (d.dependenciesState_ === IDerivationState_.UP_TO_DATE_) { d.onBecomeStale_() } @@ -219,7 +226,7 @@ export function propagateChangeConfirmed(observable: IObservable) { } observable.lowestObserverState_ = IDerivationState_.STALE_ - observable.observers_.forEach(d => { + observable.observers_?.forEach(d => { if (d.dependenciesState_ === IDerivationState_.POSSIBLY_STALE_) { d.dependenciesState_ = IDerivationState_.STALE_ } else if ( @@ -239,7 +246,7 @@ export function propagateMaybeChanged(observable: IObservable) { } observable.lowestObserverState_ = IDerivationState_.POSSIBLY_STALE_ - observable.observers_.forEach(d => { + observable.observers_?.forEach(d => { if (d.dependenciesState_ === IDerivationState_.UP_TO_DATE_) { d.dependenciesState_ = IDerivationState_.POSSIBLY_STALE_ d.onBecomeStale_() From a9086076b9ead1a9c933215bffaa9b57d44f6829 Mon Sep 17 00:00:00 2001 From: spokodev Date: Wed, 19 Aug 2026 09:55:08 +0100 Subject: [PATCH 6/6] fix(mobx): return ObservableSet set-method results in receiver order (#4681) union, intersection and symmetricDifference delegated to the argument when it was a plain Set, so results came out in the argument's order instead of the receiver's. Build every result from the receiver, as difference/isSubsetOf/isSupersetOf already do, to match native Set. --- .../fix-observableset-receiver-order.md | 5 ++++ packages/mobx/__tests__/base/set.js | 11 ++++++++ packages/mobx/src/types/observableset.ts | 28 +++---------------- 3 files changed, 20 insertions(+), 24 deletions(-) create mode 100644 .changeset/fix-observableset-receiver-order.md diff --git a/.changeset/fix-observableset-receiver-order.md b/.changeset/fix-observableset-receiver-order.md new file mode 100644 index 000000000..12a1cc135 --- /dev/null +++ b/.changeset/fix-observableset-receiver-order.md @@ -0,0 +1,5 @@ +--- +"mobx": patch +--- + +Fix ObservableSet union, intersection and symmetricDifference to return results in receiver order, matching native Set, when the argument is a plain Set. diff --git a/packages/mobx/__tests__/base/set.js b/packages/mobx/__tests__/base/set.js index 408116f0b..6d211c241 100644 --- a/packages/mobx/__tests__/base/set.js +++ b/packages/mobx/__tests__/base/set.js @@ -339,6 +339,17 @@ describe("The Set object methods do what they are supposed to do", () => { expect(isDisjointFromObservableResult).toBeTruthy() }) + test("set methods preserve receiver order (matches native Set)", () => { + const nativeCopy = new Set(reactiveSet) + const other = new Set([6, 2, 1]) + + expect([...reactiveSet.union(other)]).toEqual([...nativeCopy.union(other)]) + expect([...reactiveSet.intersection(other)]).toEqual([...nativeCopy.intersection(other)]) + expect([...reactiveSet.symmetricDifference(other)]).toEqual([ + ...nativeCopy.symmetricDifference(other) + ]) + }) + test("with ObservableSet #3919", () => { const intersectionObservableResult = reactiveSet.intersection(set([1, 2, 6])) const unionObservableResult = reactiveSet.union(set([1, 2, 6])) diff --git a/packages/mobx/src/types/observableset.ts b/packages/mobx/src/types/observableset.ts index ca088463b..7be89f061 100644 --- a/packages/mobx/src/types/observableset.ts +++ b/packages/mobx/src/types/observableset.ts @@ -233,21 +233,11 @@ export class ObservableSet implements Set, IInterceptable(otherSet: ReadonlySetLike | Set): Set { - if (isES6Set(otherSet) && !isObservableSet(otherSet)) { - return otherSet.intersection(this) - } else { - const dehancedSet = new Set(this) - return dehancedSet.intersection(otherSet) - } + return new Set(this).intersection(otherSet) } union(otherSet: ReadonlySetLike | Set): Set { - if (isES6Set(otherSet) && !isObservableSet(otherSet)) { - return otherSet.union(this) - } else { - const dehancedSet = new Set(this) - return dehancedSet.union(otherSet) - } + return new Set(this).union(otherSet) } difference(otherSet: ReadonlySetLike): Set { @@ -255,12 +245,7 @@ export class ObservableSet implements Set, IInterceptable(otherSet: ReadonlySetLike | Set): Set { - if (isES6Set(otherSet) && !isObservableSet(otherSet)) { - return otherSet.symmetricDifference(this) - } else { - const dehancedSet = new Set(this) - return dehancedSet.symmetricDifference(otherSet) - } + return new Set(this).symmetricDifference(otherSet) } isSubsetOf(otherSet: ReadonlySetLike): boolean { @@ -272,12 +257,7 @@ export class ObservableSet implements Set, IInterceptable | Set): boolean { - if (isES6Set(otherSet) && !isObservableSet(otherSet)) { - return otherSet.isDisjointFrom(this) - } else { - const dehancedSet = new Set(this) - return dehancedSet.isDisjointFrom(otherSet) - } + return new Set(this).isDisjointFrom(otherSet) } replace(other: ObservableSet | IObservableSetInitialValues): ObservableSet {