Skip to content

Commit

Permalink
chore: cleanup global state version
Browse files Browse the repository at this point in the history
  • Loading branch information
mweststrate committed Sep 23, 2023
1 parent db892d7 commit 1ee4bc9
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 84 deletions.
15 changes: 2 additions & 13 deletions packages/mobx-react-lite/src/useObserver.ts
Expand Up @@ -24,18 +24,9 @@ type ObserverAdministration = {
getSnapshot: Parameters<typeof React.useSyncExternalStore>[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?.()
})
}
Expand Down Expand Up @@ -81,9 +72,7 @@ export function useObserver<T>(render: () => T, baseComponentName: string = "obs
},
getSnapshot() {
// Do NOT access admRef here!
return globalStateVersionIsAvailable
? _getGlobalState().stateVersion
: adm.stateVersion
return adm.stateVersion
}
}

Expand Down
37 changes: 0 additions & 37 deletions packages/mobx/__tests__/v5/base/observables.js
Expand Up @@ -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(() => {})

Expand Down
19 changes: 2 additions & 17 deletions packages/mobx/src/core/atom.ts
Expand Up @@ -11,8 +11,7 @@ import {
propagateChanged,
reportObserved,
startBatch,
Lambda,
globalState
Lambda
} from "../internal"

export const $mobx = Symbol("mobx administration")
Expand All @@ -27,17 +26,14 @@ export class Atom implements IAtom {
isBeingObserved_ = false
observers_ = new Set<IDerivation>()

batchId_: number
diffValue_ = 0
lastAccessedBy_ = 0
lowestObserverState_ = IDerivationState_.NOT_TRACKING_
/**
* 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<Lambda> | undefined
Expand Down Expand Up @@ -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()
Expand Down
11 changes: 0 additions & 11 deletions packages/mobx/src/core/globalstate.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions packages/mobx/src/core/observable.ts
Expand Up @@ -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++
}

Expand Down

0 comments on commit 1ee4bc9

Please sign in to comment.