From 1ee4bc92d877cc1d7759bbfc13c843f23f48cf7c Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Sat, 23 Sep 2023 19:48:04 +0200 Subject: [PATCH] chore: cleanup global state version --- packages/mobx-react-lite/src/useObserver.ts | 15 +------- .../mobx/__tests__/v5/base/observables.js | 37 ------------------- packages/mobx/src/core/atom.ts | 19 +--------- packages/mobx/src/core/globalstate.ts | 11 ------ packages/mobx/src/core/observable.ts | 6 --- 5 files changed, 4 insertions(+), 84 deletions(-) diff --git a/packages/mobx-react-lite/src/useObserver.ts b/packages/mobx-react-lite/src/useObserver.ts index 0061d785e..c429c91e3 100644 --- a/packages/mobx-react-lite/src/useObserver.ts +++ b/packages/mobx-react-lite/src/useObserver.ts @@ -24,18 +24,9 @@ type ObserverAdministration = { getSnapshot: Parameters[1] } -// BC -const globalStateVersionIsAvailable = false // See #3728; TODO: typeof _getGlobalState().stateVersion !== "undefined" - function createReaction(adm: ObserverAdministration) { adm.reaction = new Reaction(`observer${adm.name}`, () => { - if (!globalStateVersionIsAvailable) { - // BC - adm.stateVersion = Symbol() - } - // onStoreChange won't be available until the component "mounts". - // If state changes in between initial render and mount, - // `useSyncExternalStore` should handle that by checking the state version and issuing update. + adm.stateVersion = Symbol() adm.onStoreChange?.() }) } @@ -81,9 +72,7 @@ export function useObserver(render: () => T, baseComponentName: string = "obs }, getSnapshot() { // Do NOT access admRef here! - return globalStateVersionIsAvailable - ? _getGlobalState().stateVersion - : adm.stateVersion + return adm.stateVersion } } diff --git a/packages/mobx/__tests__/v5/base/observables.js b/packages/mobx/__tests__/v5/base/observables.js index 40e28949a..24d229ac1 100644 --- a/packages/mobx/__tests__/v5/base/observables.js +++ b/packages/mobx/__tests__/v5/base/observables.js @@ -2361,43 +2361,6 @@ describe("`requiresObservable` takes precedence over global `reactionRequiresObs }) }) -test("state version updates correctly", () => { - // This test was designed around the idea of updating version only at the end of batch, - // which is NOT an implementation we've settled on, but the test is still valid. - - // This test demonstrates that the version is correctly updated with each state mutations: - // 1. Even without wrapping mutation in batch explicitely. - // 2. Even in self-invoking recursive derivation. - const o = mobx.observable({ x: 0 }) - let prevStateVersion - - const disposeAutorun = mobx.autorun(() => { - if (o.x === 5) { - disposeAutorun() - return - } - const currentStateVersion = getGlobalState().stateVersion - expect(prevStateVersion).not.toBe(currentStateVersion) - prevStateVersion = currentStateVersion - o.x++ - }) - - // expect(o.x).toBe(4) is 1? - prevStateVersion = getGlobalState().stateVersion - o.x++ - expect(o.x).toBe(5) - expect(prevStateVersion).not.toBe(getGlobalState().stateVersion) -}) - -test("Atom.reportChanged does not change state version when called from the batch the atom was created in", () => { - mobx.transaction(() => { - const prevStateVersion = getGlobalState().stateVersion - const atom = mobx.createAtom() - atom.reportChanged() - expect(prevStateVersion).toBe(getGlobalState().stateVersion) - }) -}) - test('Observables initialization does not violate `enforceActions: "always"`', () => { const consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}) diff --git a/packages/mobx/src/core/atom.ts b/packages/mobx/src/core/atom.ts index 673e7687a..29ce0213d 100644 --- a/packages/mobx/src/core/atom.ts +++ b/packages/mobx/src/core/atom.ts @@ -11,8 +11,7 @@ import { propagateChanged, reportObserved, startBatch, - Lambda, - globalState + Lambda } from "../internal" export const $mobx = Symbol("mobx administration") @@ -27,7 +26,6 @@ export class Atom implements IAtom { isBeingObserved_ = false observers_ = new Set() - batchId_: number diffValue_ = 0 lastAccessedBy_ = 0 lowestObserverState_ = IDerivationState_.NOT_TRACKING_ @@ -35,9 +33,7 @@ export class Atom implements IAtom { * Create a new atom. For debugging purposes it is recommended to give it a name. * The onBecomeObserved and onBecomeUnobserved callbacks can be used for resource management. */ - constructor(public name_ = __DEV__ ? "Atom@" + getNextId() : "Atom") { - this.batchId_ = globalState.inBatch ? globalState.batchId : NaN - } + constructor(public name_ = __DEV__ ? "Atom@" + getNextId() : "Atom") {} // onBecomeObservedListeners public onBOL: Set | undefined @@ -68,17 +64,6 @@ export class Atom implements IAtom { * Invoke this method _after_ this method has changed to signal mobx that all its observers should invalidate. */ public reportChanged() { - if (!globalState.inBatch || this.batchId_ !== globalState.batchId) { - // We could update state version only at the end of batch, - // but we would still have to switch some global flag here to signal a change. - globalState.stateVersion = - globalState.stateVersion < Number.MAX_SAFE_INTEGER - ? globalState.stateVersion + 1 - : Number.MIN_SAFE_INTEGER - // Avoids the possibility of hitting the same globalState.batchId when it cycled through all integers (necessary?) - this.batchId_ = NaN - } - startBatch() propagateChanged(this) endBatch() diff --git a/packages/mobx/src/core/globalstate.ts b/packages/mobx/src/core/globalstate.ts index 7ffa26006..35de9dfea 100644 --- a/packages/mobx/src/core/globalstate.ts +++ b/packages/mobx/src/core/globalstate.ts @@ -63,12 +63,6 @@ export class MobXGlobals { */ inBatch: number = 0 - /** - * ID of the latest batch. Used to suppress reportChanged of newly created atoms. - * Note the value persists even after batch ended. - */ - batchId: number = Number.MIN_SAFE_INTEGER - /** * Observables that don't have observers anymore, and are about to be * suspended, unless somebody else accesses it in the same batch @@ -156,11 +150,6 @@ export class MobXGlobals { * configurable: true */ safeDescriptors = true - - /** - * Changes with each state update, used by useSyncExternalStore - */ - stateVersion = Number.MIN_SAFE_INTEGER } let canMergeGlobalState = true diff --git a/packages/mobx/src/core/observable.ts b/packages/mobx/src/core/observable.ts index 0b17bbe42..f03d5b7b6 100644 --- a/packages/mobx/src/core/observable.ts +++ b/packages/mobx/src/core/observable.ts @@ -104,12 +104,6 @@ export function queueForUnobservation(observable: IObservable) { * Avoids unnecessary recalculations. */ export function startBatch() { - if (globalState.inBatch === 0) { - globalState.batchId = - globalState.batchId < Number.MAX_SAFE_INTEGER - ? globalState.batchId + 1 - : Number.MIN_SAFE_INTEGER - } globalState.inBatch++ }