(fn: () => T, baseComponentName: string = "observed"): T {
diff --git a/packages/mobx-react-lite/src/observer.ts b/packages/mobx-react-lite/src/observer.ts
index dee0bb2e5..b625a742e 100644
--- a/packages/mobx-react-lite/src/observer.ts
+++ b/packages/mobx-react-lite/src/observer.ts
@@ -104,11 +104,13 @@ export function observer(
return useObserver(() => render(props, ref), baseComponentName)
}
- // Don't set `displayName` for anonymous components,
- // so the `displayName` can be customized by user, see #3192.
- if (baseComponentName !== "") {
- ;(observerComponent as React.FunctionComponent).displayName = baseComponentName
- }
+ // Inherit original name and displayName, see #3438
+ ;(observerComponent as React.FunctionComponent).displayName = baseComponent.displayName
+ Object.defineProperty(observerComponent, "name", {
+ value: baseComponent.name,
+ writable: true,
+ configurable: true
+ })
// Support legacy context: `contextTypes` must be applied before `memo`
if ((baseComponent as any).contextTypes) {
@@ -136,7 +138,7 @@ export function observer
(
set() {
throw new Error(
`[mobx-react-lite] \`${
- this.displayName || this.type?.displayName || "Component"
+ this.displayName || this.type?.displayName || this.type?.name || "Component"
}.contextTypes\` must be set before applying \`observer\`.`
)
}
diff --git a/packages/mobx-react-lite/src/useObserver.ts b/packages/mobx-react-lite/src/useObserver.ts
index c54939520..52c9b6206 100644
--- a/packages/mobx-react-lite/src/useObserver.ts
+++ b/packages/mobx-react-lite/src/useObserver.ts
@@ -1,129 +1,115 @@
-import { Reaction } from "mobx"
+import { Reaction, _getGlobalState } from "mobx"
import React from "react"
import { printDebugValue } from "./utils/printDebugValue"
-import { observerFinalizationRegistry } from "./utils/observerFinalizationRegistry"
import { isUsingStaticRendering } from "./staticRendering"
+import { observerFinalizationRegistry } from "./utils/observerFinalizationRegistry"
+import { useSyncExternalStore } from "use-sync-external-store/shim"
-function observerComponentNameFor(baseComponentName: string) {
- return `observer${baseComponentName}`
-}
-
+// Do not store `admRef` (even as part of a closure!) on this object,
+// otherwise it will prevent GC and therefore reaction disposal via FinalizationRegistry.
type ObserverAdministration = {
- /** The Reaction created during first render, which may be leaked */
- reaction: Reaction | null
-
- /**
- * Whether the component has yet completed mounting (for us, whether
- * its useEffect has run)
- */
- mounted: boolean
-
- /**
- * Whether the observables that the component is tracking changed between
- * the first render and the first useEffect.
- */
- changedBeforeMount: boolean
+ reaction: Reaction | null // also serves as disposed flag
+ forceUpdate: Function | null // also serves as mounted flag
+ // BC: we will use local state version if global isn't available.
+ // It should behave as previous implementation - tearing is still present,
+ // because there is no cross component synchronization,
+ // but we can use `useSyncExternalStore` API.
+ stateVersion: any
+ name: string
+ // These don't depend on state/props, therefore we can keep them here instead of `useCallback`
+ subscribe: Parameters[0]
+ getSnapshot: Parameters[1]
}
-/**
- * We use class to make it easier to detect in heap snapshots by name
- */
-class ObjectToBeRetainedByReact {}
+const mobxGlobalState = _getGlobalState()
+
+// BC
+const globalStateVersionIsAvailable = typeof mobxGlobalState.globalVersion !== "undefined"
-function objectToBeRetainedByReactFactory() {
- return new ObjectToBeRetainedByReact()
+function createReaction(adm: ObserverAdministration) {
+ adm.reaction = new Reaction(`observer${adm.name}`, () => {
+ if (!globalStateVersionIsAvailable) {
+ // BC
+ adm.stateVersion = Symbol()
+ }
+ // Force update won't be avaliable 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.forceUpdate?.()
+ })
}
-export function useObserver(fn: () => T, baseComponentName: string = "observed"): T {
+export function useObserver(render: () => T, baseComponentName: string = "observed"): T {
if (isUsingStaticRendering()) {
- return fn()
+ return render()
}
- const [objectRetainedByReact] = React.useState(objectToBeRetainedByReactFactory)
- // Force update, see #2982
- const [, setState] = React.useState()
- const forceUpdate = () => setState([] as any)
-
- // StrictMode/ConcurrentMode/Suspense may mean that our component is
- // rendered and abandoned multiple times, so we need to track leaked
- // Reactions.
const admRef = React.useRef(null)
if (!admRef.current) {
// First render
- admRef.current = {
+ const adm: ObserverAdministration = {
reaction: null,
- mounted: false,
- changedBeforeMount: false
+ forceUpdate: null,
+ stateVersion: Symbol(),
+ name: baseComponentName,
+ subscribe(onStoreChange: () => void) {
+ // Do NOT access admRef here!
+ observerFinalizationRegistry.unregister(adm)
+ adm.forceUpdate = onStoreChange
+ if (!adm.reaction) {
+ // We've lost our reaction and therefore all subscriptions.
+ // We have to recreate reaction and schedule re-render to recreate subscriptions,
+ // even if state did not change.
+ createReaction(adm)
+ adm.forceUpdate()
+ }
+
+ return () => {
+ // Do NOT access admRef here!
+ adm.forceUpdate = null
+ adm.reaction?.dispose()
+ adm.reaction = null
+ }
+ },
+ getSnapshot() {
+ // Do NOT access admRef here!
+ return globalStateVersionIsAvailable
+ ? mobxGlobalState.stateVersion
+ : adm.stateVersion
+ }
}
+
+ admRef.current = adm
}
const adm = admRef.current!
if (!adm.reaction) {
- // First render or component was not committed and reaction was disposed by registry
- adm.reaction = new Reaction(observerComponentNameFor(baseComponentName), () => {
- // Observable has changed, meaning we want to re-render
- // BUT if we're a component that hasn't yet got to the useEffect()
- // stage, we might be a component that _started_ to render, but
- // got dropped, and we don't want to make state changes then.
- // (It triggers warnings in StrictMode, for a start.)
- if (adm.mounted) {
- // We have reached useEffect(), so we're mounted, and can trigger an update
- forceUpdate()
- } else {
- // We haven't yet reached useEffect(), so we'll need to trigger a re-render
- // when (and if) useEffect() arrives.
- adm.changedBeforeMount = true
- }
- })
-
- observerFinalizationRegistry.register(objectRetainedByReact, adm, adm)
+ // First render or reaction was disposed by registry before subscribe
+ createReaction(adm)
+ // StrictMode/ConcurrentMode/Suspense may mean that our component is
+ // rendered and abandoned multiple times, so we need to track leaked
+ // Reactions.
+ observerFinalizationRegistry.register(admRef, adm, adm)
}
- React.useDebugValue(adm.reaction, printDebugValue)
-
- React.useEffect(() => {
- observerFinalizationRegistry.unregister(adm)
-
- adm.mounted = true
+ React.useDebugValue(adm.reaction!, printDebugValue)
- if (adm.reaction) {
- if (adm.changedBeforeMount) {
- // Got a change before mount, force an update
- adm.changedBeforeMount = false
- forceUpdate()
- }
- } else {
- // The reaction we set up in our render has been disposed.
- // This can be due to bad timings of renderings, e.g. our
- // component was paused for a _very_ long time, and our
- // reaction got cleaned up
-
- // Re-create the reaction
- adm.reaction = new Reaction(observerComponentNameFor(baseComponentName), () => {
- // We've definitely already been mounted at this point
- forceUpdate()
- })
- forceUpdate()
- }
-
- return () => {
- adm.reaction!.dispose()
- adm.reaction = null
- adm.mounted = false
- adm.changedBeforeMount = false
- }
- }, [])
+ useSyncExternalStore(
+ // Both of these must be stable, otherwise it would keep resubscribing every render.
+ adm.subscribe,
+ adm.getSnapshot
+ )
// render the original component, but have the
// reaction track the observables, so that rendering
// can be invalidated (see above) once a dependency changes
- let rendering!: T
+ let renderResult!: T
let exception
- adm.reaction.track(() => {
+ adm.reaction!.track(() => {
try {
- rendering = fn()
+ renderResult = render()
} catch (e) {
exception = e
}
@@ -133,5 +119,5 @@ export function useObserver(fn: () => T, baseComponentName: string = "observe
throw exception // re-throw any exceptions caught during rendering
}
- return rendering
+ return renderResult
}
diff --git a/packages/mobx-react/__tests__/__snapshots__/observer.test.tsx.snap b/packages/mobx-react/__tests__/__snapshots__/observer.test.tsx.snap
index 0dd2351e4..3c972cb79 100644
--- a/packages/mobx-react/__tests__/__snapshots__/observer.test.tsx.snap
+++ b/packages/mobx-react/__tests__/__snapshots__/observer.test.tsx.snap
@@ -1,22 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
-exports[`#797 - replacing this.render should trigger a warning 1`] = `
-[MockFunction] {
- "calls": Array [
- Array [
- "The reactive render of an observer class component (Component)
- was overridden after MobX attached. This may result in a memory leak if the
- overridden reactive render was not properly disposed.",
- ],
- ],
- "results": Array [
- Object {
- "type": "return",
- "value": undefined,
- },
- ],
-}
-`;
+exports[`#3492 should not cause warning by calling forceUpdate on uncommited components 1`] = `[MockFunction]`;
exports[`Redeclaring an existing observer component as an observer should log a warning 1`] = `
[MockFunction] {
@@ -35,23 +19,7 @@ exports[`Redeclaring an existing observer component as an observer should log a
}
`;
-exports[`SSR works #3448 1`] = `
-[MockFunction] {
- "calls": Array [
- Array [
- "The reactive render of an observer class component (TestCmp)
- was overridden after MobX attached. This may result in a memory leak if the
- overridden reactive render was not properly disposed.",
- ],
- ],
- "results": Array [
- Object {
- "type": "return",
- "value": undefined,
- },
- ],
-}
-`;
+exports[`SSR works #3448 1`] = `[MockFunction]`;
exports[`issue 12 1`] = `
diff --git a/packages/mobx-react/__tests__/finalizationRegistry.tsx b/packages/mobx-react/__tests__/finalizationRegistry.tsx
new file mode 100644
index 000000000..76673a5b5
--- /dev/null
+++ b/packages/mobx-react/__tests__/finalizationRegistry.tsx
@@ -0,0 +1,88 @@
+import { cleanup, render, waitFor } from "@testing-library/react"
+import * as mobx from "mobx"
+import * as React from "react"
+
+// @ts-ignore
+import gc from "expose-gc/function"
+import { observer } from "../src"
+
+afterEach(cleanup)
+
+function sleep(time: number) {
+ return new Promise
(res => {
+ setTimeout(res, time)
+ })
+}
+
+// TODO remove once https://github.com/mobxjs/mobx/pull/3620 is merged.
+declare class WeakRef {
+ constructor(object: T)
+ deref(): T | undefined
+}
+
+test("should not prevent GC of uncomitted components", async () => {
+ expect(typeof globalThis.FinalizationRegistry).toBe("function")
+
+ // This specific setup causes first instance of A not being commited.
+ // This is checked by comparing constructor and componentDidMount invocation counts.
+ // There is no profound reason why that's the case, if you know a simpler or more robust setup
+ // feel free to change this.
+
+ const o = mobx.observable({ x: 0 })
+ let aConstructorCount = 0
+ let aMountCount = 0
+
+ let firstARef: WeakRef
+
+ @observer
+ class A extends React.Component {
+ constructor(props) {
+ super(props)
+ if (aConstructorCount === 0) {
+ firstARef = new WeakRef(this)
+ }
+ aConstructorCount++
+ }
+ componentDidMount(): void {
+ aMountCount++
+ }
+ render() {
+ return (
+
+
+ {o.x}
+
+ )
+ }
+ }
+
+ class B extends React.Component {
+ render() {
+ return "B"
+ }
+ }
+
+ const LazyA = React.lazy(() => Promise.resolve({ default: A }))
+ const LazyB = React.lazy(() => Promise.resolve({ default: B }))
+
+ function App() {
+ return (
+
+
+
+ )
+ }
+
+ const { unmount, container } = render()
+
+ expect(container).toHaveTextContent("fallback")
+ await waitFor(() => expect(container).toHaveTextContent("B0"))
+ expect(aConstructorCount).toBe(2)
+ expect(aMountCount).toBe(1)
+
+ gc()
+ await sleep(50)
+ expect(firstARef!.deref()).toBeUndefined()
+
+ unmount()
+})
diff --git a/packages/mobx-react/__tests__/inject.test.tsx b/packages/mobx-react/__tests__/inject.test.tsx
index 3259b5bbb..c2fdeaf85 100644
--- a/packages/mobx-react/__tests__/inject.test.tsx
+++ b/packages/mobx-react/__tests__/inject.test.tsx
@@ -102,7 +102,7 @@ describe("inject based context", () => {
expect(C.displayName).toBe("inject(ComponentC)")
})
- test.only("shouldn't change original displayName of component that uses forwardRef", () => {
+ test("shouldn't change original displayName of component that uses forwardRef", () => {
const FancyComp = React.forwardRef((_: any, ref: React.Ref) => {
return
})
@@ -498,12 +498,21 @@ describe("inject based context", () => {
expect(injectRender).toBe(6)
expect(itemRender).toBe(6)
- container.querySelectorAll(".hl_ItemB").forEach((e: Element) => (e as HTMLElement).click())
+ act(() => {
+ container
+ .querySelectorAll(".hl_ItemB")
+ .forEach((e: Element) => (e as HTMLElement).click())
+ })
+
expect(listRender).toBe(1)
expect(injectRender).toBe(12) // ideally, 7
expect(itemRender).toBe(7)
+ act(() => {
+ container
+ .querySelectorAll(".hl_ItemF")
+ .forEach((e: Element) => (e as HTMLElement).click())
+ })
- container.querySelectorAll(".hl_ItemF").forEach((e: Element) => (e as HTMLElement).click())
expect(listRender).toBe(1)
expect(injectRender).toBe(18) // ideally, 9
expect(itemRender).toBe(9)
diff --git a/packages/mobx-react/__tests__/observer.test.tsx b/packages/mobx-react/__tests__/observer.test.tsx
index b4b0285d3..cc5d818d8 100644
--- a/packages/mobx-react/__tests__/observer.test.tsx
+++ b/packages/mobx-react/__tests__/observer.test.tsx
@@ -1,6 +1,6 @@
-import React, { createContext, StrictMode } from "react"
-import { inject, observer, Observer, enableStaticRendering, useStaticRendering } from "../src"
-import { render, act } from "@testing-library/react"
+import React, { createContext, Fragment, StrictMode, Suspense } from "react"
+import { inject, observer, Observer, enableStaticRendering } from "../src"
+import { render, act, waitFor } from "@testing-library/react"
import {
getObserverTree,
_resetGlobalState,
@@ -353,7 +353,8 @@ test("correctly wraps display name of child component", () => {
})
expect(A.name).toEqual("ObserverClass")
- expect((B as any).type.displayName).toEqual("StatelessObserver")
+ expect((B as any).type.name).toEqual("StatelessObserver")
+ expect((B as any).type.displayName).toEqual(undefined)
})
describe("124 - react to changes in this.props via computed", () => {
@@ -870,29 +871,6 @@ test.skip("#709 - applying observer on React.memo component", () => {
render(, { wrapper: ErrorCatcher })
})
-test("#797 - replacing this.render should trigger a warning", () => {
- consoleWarnMock = jest.spyOn(console, "warn").mockImplementation(() => {})
-
- @observer
- class Component extends React.Component {
- render() {
- return
- }
- swapRenderFunc() {
- this.render = () => {
- return
- }
- }
- }
-
- const compRef = React.createRef()
- const { unmount } = render()
- compRef.current?.swapRenderFunc()
- unmount()
-
- expect(consoleWarnMock).toMatchSnapshot()
-})
-
test("Redeclaring an existing observer component as an observer should log a warning", () => {
consoleWarnMock = jest.spyOn(console, "warn").mockImplementation(() => {})
@@ -1032,8 +1010,69 @@ test("SSR works #3448", () => {
enableStaticRendering(true)
const { unmount, container } = render(app)
expect(container).toHaveTextContent(":)")
- enableStaticRendering(false)
unmount()
+ enableStaticRendering(false)
+
+ expect(consoleWarnMock).toMatchSnapshot()
+})
+
+test("#3492 should not cause warning by calling forceUpdate on uncommited components", async () => {
+ consoleWarnMock = jest.spyOn(console, "warn").mockImplementation(() => {})
+
+ const o = observable({ x: 0 })
+ let aConstructorCount = 0
+ let aMountCount = 0
+ let aRenderCount = 0
+ @observer
+ class A extends React.Component {
+ constructor(props) {
+ super(props)
+ aConstructorCount++
+ }
+ componentDidMount(): void {
+ aMountCount++
+ }
+ render() {
+ aRenderCount++
+ return (
+
+
+ {o.x}
+
+ )
+ }
+ }
+
+ class B extends React.Component {
+ render() {
+ return "B"
+ }
+ }
+
+ const LazyA = React.lazy(() => Promise.resolve({ default: A }))
+ const LazyB = React.lazy(() => Promise.resolve({ default: B }))
+
+ function App() {
+ return (
+
+
+
+ )
+ }
+
+ const { unmount, container } = render()
+
+ expect(container).toHaveTextContent("fallback")
+ await waitFor(() => expect(container).toHaveTextContent("B0"))
+ act(() => {
+ o.x++
+ })
+ expect(container).toHaveTextContent("B1")
+ // React throws away the first instance, therefore the mismatch
+ expect(aConstructorCount).toBe(2)
+ expect(aMountCount).toBe(1)
+ expect(aRenderCount).toBe(3)
+ unmount()
expect(consoleWarnMock).toMatchSnapshot()
})
diff --git a/packages/mobx-react/__tests__/symbol.test.tsx b/packages/mobx-react/__tests__/symbol.test.tsx
deleted file mode 100644
index 4fe06ba45..000000000
--- a/packages/mobx-react/__tests__/symbol.test.tsx
+++ /dev/null
@@ -1,25 +0,0 @@
-import React from "react"
-import { observer } from "../src"
-import { render } from "@testing-library/react"
-import { newSymbol } from "../src/utils/utils"
-
-// @ts-ignore
-delete global.Symbol
-
-test("work without Symbol", () => {
- const Component1 = observer(
- class extends React.Component {
- render() {
- return null
- }
- }
- )
- render()
-})
-
-test("cache newSymbol created Symbols", () => {
- const symbol1 = newSymbol("name")
- const symbol2 = newSymbol("name")
-
- expect(symbol1).toEqual(symbol2)
-})
diff --git a/packages/mobx-react/package.json b/packages/mobx-react/package.json
index 03f7de0d9..479fe6a7d 100644
--- a/packages/mobx-react/package.json
+++ b/packages/mobx-react/package.json
@@ -36,10 +36,10 @@
},
"homepage": "https://mobx.js.org",
"dependencies": {
- "mobx-react-lite": "^3.4.0"
+ "mobx-react-lite": "^3.4.3"
},
"peerDependencies": {
- "mobx": "^6.1.0",
+ "mobx": "^6.9.0",
"react": "^16.8.0 || ^17 || ^18"
},
"peerDependenciesMeta": {
@@ -51,8 +51,9 @@
}
},
"devDependencies": {
- "mobx": "^6.7.0",
- "mobx-react-lite": "^3.4.0"
+ "mobx": "^6.8.0",
+ "mobx-react-lite": "^3.4.3",
+ "expose-gc": "^1.0.0"
},
"keywords": [
"mobx",
diff --git a/packages/mobx-react/src/disposeOnUnmount.ts b/packages/mobx-react/src/disposeOnUnmount.ts
index 409273d6f..fb5d7e251 100644
--- a/packages/mobx-react/src/disposeOnUnmount.ts
+++ b/packages/mobx-react/src/disposeOnUnmount.ts
@@ -1,10 +1,13 @@
import React from "react"
-import { patch, newSymbol } from "./utils/utils"
+import { patch } from "./utils/utils"
+
+const reactMajorVersion = Number.parseInt(React.version.split(".")[0])
+let warnedAboutDisposeOnUnmountDeprecated = false
type Disposer = () => void
-const protoStoreKey = newSymbol("disposeOnUnmountProto")
-const instStoreKey = newSymbol("disposeOnUnmountInst")
+const protoStoreKey = Symbol("disposeOnUnmountProto")
+const instStoreKey = Symbol("disposeOnUnmountInst")
function runDisposersOnWillUnmount() {
;[...(this[protoStoreKey] || []), ...(this[instStoreKey] || [])].forEach(propKeyOrFunction => {
@@ -17,12 +20,22 @@ function runDisposersOnWillUnmount() {
})
}
+/**
+ * @deprecated `disposeOnUnmount` is not compatible with React 18 and higher.
+ */
export function disposeOnUnmount(target: React.Component, propertyKey: PropertyKey): void
+
+/**
+ * @deprecated `disposeOnUnmount` is not compatible with React 18 and higher.
+ */
export function disposeOnUnmount>(
target: React.Component,
fn: TF
): TF
+/**
+ * @deprecated `disposeOnUnmount` is not compatible with React 18 and higher.
+ */
export function disposeOnUnmount(
target: React.Component,
propertyKeyOrFunction: PropertyKey | Disposer | Array
@@ -31,6 +44,19 @@ export function disposeOnUnmount(
return propertyKeyOrFunction.map(fn => disposeOnUnmount(target, fn))
}
+ if (!warnedAboutDisposeOnUnmountDeprecated) {
+ if (reactMajorVersion >= 18) {
+ console.error(
+ "[mobx-react] disposeOnUnmount is not compatible with React 18 and higher. Don't use it."
+ )
+ } else {
+ console.warn(
+ "[mobx-react] disposeOnUnmount is deprecated. It won't work correctly with React 18 and higher."
+ )
+ }
+ warnedAboutDisposeOnUnmountDeprecated = true
+ }
+
const c = Object.getPrototypeOf(target).constructor
const c2 = Object.getPrototypeOf(target.constructor)
// Special case for react-hot-loader
diff --git a/packages/mobx-react/src/index.ts b/packages/mobx-react/src/index.ts
index 6bdd97929..65abc7ed7 100644
--- a/packages/mobx-react/src/index.ts
+++ b/packages/mobx-react/src/index.ts
@@ -1,8 +1,13 @@
import { observable } from "mobx"
import { Component } from "react"
-if (!Component) throw new Error("mobx-react requires React to be available")
-if (!observable) throw new Error("mobx-react requires mobx to be available")
+if (!Component) {
+ throw new Error("mobx-react requires React to be available")
+}
+
+if (!observable) {
+ throw new Error("mobx-react requires mobx to be available")
+}
export {
Observer,
diff --git a/packages/mobx-react/src/observerClass.ts b/packages/mobx-react/src/observerClass.ts
index 7a2c6be1a..dd91434a6 100644
--- a/packages/mobx-react/src/observerClass.ts
+++ b/packages/mobx-react/src/observerClass.ts
@@ -1,44 +1,80 @@
-import { PureComponent, Component } from "react"
+import { PureComponent, Component, ComponentClass, ClassAttributes } from "react"
import {
createAtom,
_allowStateChanges,
Reaction,
- $mobx,
_allowStateReadsStart,
- _allowStateReadsEnd
+ _allowStateReadsEnd,
+ _getGlobalState,
+ IAtom
} from "mobx"
-import { isUsingStaticRendering } from "mobx-react-lite"
+import {
+ isUsingStaticRendering,
+ _observerFinalizationRegistry as observerFinalizationRegistry
+} from "mobx-react-lite"
+import { shallowEqual, patch } from "./utils/utils"
+
+const administrationSymbol = Symbol("ObserverAdministration")
+const isMobXReactObserverSymbol = Symbol("isMobXReactObserver")
-import { newSymbol, shallowEqual, setHiddenProp, patch } from "./utils/utils"
+type ObserverAdministration = {
+ reaction: Reaction | null // also serves as disposed flag
+ forceUpdate: Function | null
+ mounted: boolean // we could use forceUpdate as mounted flag
+ name: string
+ propsAtom: IAtom
+ stateAtom: IAtom
+ contextAtom: IAtom
+ props: any
+ state: any
+ context: any
+ // Setting this.props causes forceUpdate, because this.props is observable.
+ // forceUpdate sets this.props.
+ // This flag is used to avoid the loop.
+ isUpdating: boolean
+}
-const mobxAdminProperty = $mobx || "$mobx" // BC
-const mobxObserverProperty = newSymbol("isMobXReactObserver")
-const mobxIsUnmounted = newSymbol("isUnmounted")
-const skipRenderKey = newSymbol("skipRender")
-const isForcingUpdateKey = newSymbol("isForcingUpdate")
+function getAdministration(component: Component): ObserverAdministration {
+ // We create administration lazily, because we can't patch constructor
+ // and the exact moment of initialization partially depends on React internals.
+ // At the time of writing this, the first thing invoked is one of the observable getter/setter (state/props/context).
+ return (component[administrationSymbol] ??= {
+ reaction: null,
+ mounted: false,
+ forceUpdate: null,
+ name: getDisplayName(component.constructor as ComponentClass),
+ state: undefined,
+ props: undefined,
+ context: undefined,
+ propsAtom: createAtom("props"),
+ stateAtom: createAtom("state"),
+ contextAtom: createAtom("context"),
+ isUpdating: false
+ })
+}
export function makeClassComponentObserver(
- componentClass: React.ComponentClass
-): React.ComponentClass {
- const target = componentClass.prototype
+ componentClass: ComponentClass
+): ComponentClass {
+ const { prototype } = componentClass
- if (componentClass[mobxObserverProperty]) {
- const displayName = getDisplayName(target)
+ if (componentClass[isMobXReactObserverSymbol]) {
+ const displayName = getDisplayName(componentClass)
console.warn(
`The provided component class (${displayName})
has already been declared as an observer component.`
)
} else {
- componentClass[mobxObserverProperty] = true
+ componentClass[isMobXReactObserverSymbol] = true
}
- if (target.componentWillReact) {
+ if (prototype.componentWillReact) {
throw new Error("The componentWillReact life-cycle event is no longer supported")
}
if (componentClass["__proto__"] !== PureComponent) {
- if (!target.shouldComponentUpdate) {
- target.shouldComponentUpdate = observerSCU
- } else if (target.shouldComponentUpdate !== observerSCU) {
+ if (!prototype.shouldComponentUpdate) {
+ prototype.shouldComponentUpdate = observerSCU
+ } else if (prototype.shouldComponentUpdate !== observerSCU) {
// n.b. unequal check, instead of existence check, as @observer might be on superclass as well
throw new Error(
"It is not allowed to use shouldComponentUpdate in observer based components."
@@ -50,142 +86,154 @@ export function makeClassComponentObserver(
// are defined inside the component, and which rely on state or props, re-compute if state or props change
// (otherwise the computed wouldn't update and become stale on props change, since props are not observable)
// However, this solution is not without it's own problems: https://github.com/mobxjs/mobx-react/issues?utf8=%E2%9C%93&q=is%3Aissue+label%3Aobservable-props-or-not+
- makeObservableProp(target, "props")
- makeObservableProp(target, "state")
- if (componentClass.contextType) {
- makeObservableProp(target, "context")
- }
+ Object.defineProperties(prototype, {
+ props: observablePropsDescriptor,
+ state: observableStateDescriptor,
+ context: observableContextDescriptor
+ })
- const originalRender = target.render
+ const originalRender = prototype.render
if (typeof originalRender !== "function") {
- const displayName = getDisplayName(target)
+ const displayName = getDisplayName(componentClass)
throw new Error(
`[mobx-react] class component (${displayName}) is missing \`render\` method.` +
`\n\`observer\` requires \`render\` being a function defined on prototype.` +
`\n\`render = () => {}\` or \`render = function() {}\` is not supported.`
)
}
- target.render = function () {
- this.render = isUsingStaticRendering()
- ? originalRender
- : createReactiveRender.call(this, originalRender)
+
+ prototype.render = function () {
+ Object.defineProperty(this, "render", {
+ // There is no safe way to replace render, therefore it's forbidden.
+ configurable: false,
+ writable: false,
+ value: isUsingStaticRendering()
+ ? originalRender
+ : createReactiveRender.call(this, originalRender)
+ })
return this.render()
}
- patch(target, "componentDidMount", function () {
- this[mobxIsUnmounted] = false
- if (!this.render[mobxAdminProperty]) {
- // Reaction is re-created automatically during render, but a component can re-mount and skip render #3395.
- // To re-create the reaction and re-subscribe to relevant observables we have to force an update.
- Component.prototype.forceUpdate.call(this)
+
+ patch(prototype, "componentDidMount", function () {
+ // `componentDidMount` may not be called at all. React can abandon the instance after `render`.
+ // That's why we use finalization registry to dispose reaction created during render.
+ // Happens with `` see #3492
+ //
+ // `componentDidMount` can be called immediately after `componentWillUnmount` without calling `render` in between.
+ // Happens with ``see #3395.
+ //
+ // If `componentDidMount` is called, it's guaranteed to run synchronously with render (similary to `useLayoutEffect`).
+ // Therefore we don't have to worry about external (observable) state being updated before mount (no state version checking).
+ //
+ // Things may change: "In the future, React will provide a feature that lets components preserve state between unmounts"
+
+ const admin = getAdministration(this)
+
+ admin.mounted = true
+
+ // Component instance committed, prevent reaction disposal.
+ observerFinalizationRegistry.unregister(admin)
+
+ // We don't set forceUpdate before mount because it requires a reference to `this`,
+ // therefore `this` could NOT be garbage collected before mount,
+ // preventing reaction disposal by FinalizationRegistry and leading to memory leak.
+ // As an alternative we could have `admin.instanceRef = new WeakRef(this)`, but lets avoid it if possible.
+ admin.forceUpdate = () => this.forceUpdate()
+
+ if (!admin.reaction) {
+ // 1. Instance was unmounted (reaction disposed) and immediately remounted without running render #3395.
+ // 2. Reaction was disposed by finalization registry before mount. Shouldn't ever happen for class components:
+ // `componentDidMount` runs synchronously after render, but our registry are deferred (can't run in between).
+ // In any case we lost subscriptions to observables, so we have to create new reaction and re-render to resubscribe.
+ // The reaction will be created lazily by following render.
+ admin.forceUpdate()
}
})
- patch(target, "componentWillUnmount", function () {
+
+ patch(prototype, "componentWillUnmount", function () {
if (isUsingStaticRendering()) {
return
}
-
- const reaction = this.render[mobxAdminProperty]
- if (reaction) {
- reaction.dispose()
- // Forces reaction to be re-created on next render
- this.render[mobxAdminProperty] = null
- } else {
- // Render may have been hot-swapped and/or overridden by a subclass.
- const displayName = getDisplayName(this)
- console.warn(
- `The reactive render of an observer class component (${displayName})
- was overridden after MobX attached. This may result in a memory leak if the
- overridden reactive render was not properly disposed.`
- )
- }
-
- this[mobxIsUnmounted] = true
+ const admin = getAdministration(this)
+ admin.reaction?.dispose()
+ admin.reaction = null
+ admin.forceUpdate = null
+ admin.mounted = false
})
+
return componentClass
}
// Generates a friendly name for debugging
-function getDisplayName(comp: any) {
- return (
- comp.displayName ||
- comp.name ||
- (comp.constructor && (comp.constructor.displayName || comp.constructor.name)) ||
- ""
- )
+function getDisplayName(componentClass: ComponentClass) {
+ return componentClass.displayName || componentClass.name || ""
}
function createReactiveRender(originalRender: any) {
- /**
- * If props are shallowly modified, react will render anyway,
- * so atom.reportChanged() should not result in yet another re-render
- */
- setHiddenProp(this, skipRenderKey, false)
- /**
- * forceUpdate will re-assign this.props. We don't want that to cause a loop,
- * so detect these changes
- */
- setHiddenProp(this, isForcingUpdateKey, false)
-
- const initialName = getDisplayName(this)
const boundOriginalRender = originalRender.bind(this)
- let isRenderingPending = false
-
- const createReaction = () => {
- const reaction = new Reaction(`${initialName}.render()`, () => {
- if (!isRenderingPending) {
- // N.B. Getting here *before mounting* means that a component constructor has side effects (see the relevant test in misc.test.tsx)
- // This unidiomatic React usage but React will correctly warn about this so we continue as usual
- // See #85 / Pull #44
- isRenderingPending = true
- if (this[mobxIsUnmounted] !== true) {
- let hasError = true
- try {
- setHiddenProp(this, isForcingUpdateKey, true)
- if (!this[skipRenderKey]) {
- Component.prototype.forceUpdate.call(this)
- }
- hasError = false
- } finally {
- setHiddenProp(this, isForcingUpdateKey, false)
- if (hasError) {
- reaction.dispose()
- // Forces reaction to be re-created on next render
- this.render[mobxAdminProperty] = null
- }
- }
- }
- }
- })
- reaction["reactComponent"] = this
- return reaction
- }
+ const admin = getAdministration(this)
function reactiveRender() {
- isRenderingPending = false
- // Create reaction lazily to support re-mounting #3395
- const reaction = (reactiveRender[mobxAdminProperty] ??= createReaction())
- let exception: unknown = undefined
- let rendering = undefined
- reaction.track(() => {
+ if (!admin.reaction) {
+ // Create reaction lazily to support re-mounting #3395
+ admin.reaction = createReaction(admin)
+ if (!admin.mounted) {
+ // React can abandon this instance and never call `componentDidMount`/`componentWillUnmount`,
+ // we have to make sure reaction will be disposed.
+ observerFinalizationRegistry.register(this, admin, this)
+ }
+ }
+
+ let error: unknown = undefined
+ let renderResult = undefined
+ admin.reaction.track(() => {
try {
// TODO@major
// Optimization: replace with _allowStateChangesStart/End (not available in mobx@6.0.0)
- rendering = _allowStateChanges(false, boundOriginalRender)
+ renderResult = _allowStateChanges(false, boundOriginalRender)
} catch (e) {
- exception = e
+ error = e
}
})
- if (exception) {
- throw exception
+ if (error) {
+ throw error
}
- return rendering
+ return renderResult
}
return reactiveRender
}
-function observerSCU(nextProps: React.ClassAttributes, nextState: any): boolean {
+function createReaction(admin: ObserverAdministration) {
+ return new Reaction(`${admin.name}.render()`, () => {
+ if (admin.isUpdating) {
+ // Reaction is suppressed when setting new state/props/context,
+ // this is when component is already being updated.
+ return
+ }
+
+ if (!admin.mounted) {
+ // This is neccessary to avoid react warning about calling forceUpdate on component that isn't mounted yet.
+ // This happens when component is abandoned after render - our reaction is already created and reacts to changes.
+ // Due to the synchronous nature of `componenDidMount`, we don't have to worry that component could eventually mount and require update.
+ return
+ }
+
+ try {
+ // forceUpdate sets new `props`, since we made it observable, it would `reportChanged`, causing a loop.
+ admin.isUpdating = true
+ admin.forceUpdate?.()
+ } catch (error) {
+ admin.reaction?.dispose()
+ admin.reaction = null
+ } finally {
+ admin.isUpdating = false
+ }
+ })
+}
+
+function observerSCU(nextProps: ClassAttributes, nextState: any): boolean {
if (isUsingStaticRendering()) {
console.warn(
"[mobx-react] It seems that a re-rendering of a React component is triggered while in static (server-side) mode. Please make sure components are rendered only once server-side."
@@ -202,45 +250,41 @@ function observerSCU(nextProps: React.ClassAttributes, nextState: any): boo
return !shallowEqual(this.props, nextProps)
}
-function makeObservableProp(target: any, propName: string): void {
- const valueHolderKey = newSymbol(`reactProp_${propName}_valueHolder`)
- const atomHolderKey = newSymbol(`reactProp_${propName}_atomHolder`)
- function getAtom() {
- if (!this[atomHolderKey]) {
- setHiddenProp(this, atomHolderKey, createAtom("reactive " + propName))
- }
- return this[atomHolderKey]
- }
- Object.defineProperty(target, propName, {
+function createObservablePropDescriptor(key: "props" | "state" | "context") {
+ const atomKey = `${key}Atom`
+ return {
configurable: true,
enumerable: true,
- get: function () {
- let prevReadState = false
+ get() {
+ const admin = getAdministration(this)
- // Why this check? BC?
- // @ts-expect-error
- if (_allowStateReadsStart && _allowStateReadsEnd) {
- prevReadState = _allowStateReadsStart(true)
- }
- getAtom.call(this).reportObserved()
+ let prevReadState = _allowStateReadsStart(true)
- // Why this check? BC?
- // @ts-expect-error
- if (_allowStateReadsStart && _allowStateReadsEnd) {
- _allowStateReadsEnd(prevReadState)
- }
+ admin[atomKey].reportObserved()
+
+ _allowStateReadsEnd(prevReadState)
- return this[valueHolderKey]
+ return admin[key]
},
- set: function set(v) {
- if (!this[isForcingUpdateKey] && !shallowEqual(this[valueHolderKey], v)) {
- setHiddenProp(this, valueHolderKey, v)
- setHiddenProp(this, skipRenderKey, true)
- getAtom.call(this).reportChanged()
- setHiddenProp(this, skipRenderKey, false)
+ set(value) {
+ const admin = getAdministration(this)
+ // forceUpdate issued by reaction sets new props.
+ // It sets isUpdating to true to prevent loop.
+ if (!admin.isUpdating && !shallowEqual(admin[key], value)) {
+ admin[key] = value
+ // This notifies all observers including our component,
+ // but we don't want to cause `forceUpdate`, because component is already updating,
+ // therefore supress component reaction.
+ admin.isUpdating = true
+ admin[atomKey].reportChanged()
+ admin.isUpdating = false
} else {
- setHiddenProp(this, valueHolderKey, v)
+ admin[key] = value
}
}
- })
+ }
}
+
+const observablePropsDescriptor = createObservablePropDescriptor("props")
+const observableStateDescriptor = createObservablePropDescriptor("state")
+const observableContextDescriptor = createObservablePropDescriptor("context")
diff --git a/packages/mobx-react/src/utils/utils.ts b/packages/mobx-react/src/utils/utils.ts
index c79efa456..9571d3b6f 100644
--- a/packages/mobx-react/src/utils/utils.ts
+++ b/packages/mobx-react/src/utils/utils.ts
@@ -1,21 +1,3 @@
-let symbolId = 0
-function createSymbol(name: string): symbol | string {
- if (typeof Symbol === "function") {
- return Symbol(name)
- }
- const symbol = `__$mobx-react ${name} (${symbolId})`
- symbolId++
- return symbol
-}
-
-const createdSymbols = {}
-export function newSymbol(name: string): symbol | string {
- if (!createdSymbols[name]) {
- createdSymbols[name] = createSymbol(name)
- }
- return createdSymbols[name]
-}
-
export function shallowEqual(objA: any, objB: any): boolean {
//From: https://github.com/facebook/fbjs/blob/c69904a511b900266935168223063dd8772dfc40/packages/fbjs/src/core/shallowEqual.js
if (is(objA, objB)) {
@@ -96,8 +78,8 @@ export function setHiddenProp(target: object, prop: any, value: any): void {
* Utilities for patching componentWillUnmount, to make sure @disposeOnUnmount works correctly icm with user defined hooks
* and the handler provided by mobx-react
*/
-const mobxMixins = newSymbol("patchMixins")
-const mobxPatchedDefinition = newSymbol("patchedDefinition")
+const mobxMixins = Symbol("patchMixins")
+const mobxPatchedDefinition = Symbol("patchedDefinition")
export interface Mixins extends Record {
locks: number
@@ -175,6 +157,7 @@ function createDefinition(
let wrappedFunc = wrapFunction(originalMethod, mixins)
return {
+ // @ts-ignore
[mobxPatchedDefinition]: true,
get: function () {
return wrappedFunc
diff --git a/packages/mobx/CHANGELOG.md b/packages/mobx/CHANGELOG.md
index bff23467b..bbd354cc1 100644
--- a/packages/mobx/CHANGELOG.md
+++ b/packages/mobx/CHANGELOG.md
@@ -1319,7 +1319,7 @@ A deprecation message will now be printed if creating computed properties while
```javascript
const x = observable({
- computedProp: function() {
+ computedProp: function () {
return someComputation
}
})
@@ -1344,7 +1344,7 @@ or alternatively:
```javascript
observable({
- computedProp: computed(function() {
+ computedProp: computed(function () {
return someComputation
})
})
@@ -1362,7 +1362,7 @@ N.B. If you want to introduce actions on an observable that modify its state, us
```javascript
observable({
counter: 0,
- increment: action(function() {
+ increment: action(function () {
this.counter++
})
})
@@ -1488,10 +1488,10 @@ function Square() {
extendObservable(this, {
length: 2,
squared: computed(
- function() {
+ function () {
return this.squared * this.squared
},
- function(surfaceSize) {
+ function (surfaceSize) {
this.length = Math.sqrt(surfaceSize)
}
)
diff --git a/packages/mobx/__tests__/v5/base/observables.js b/packages/mobx/__tests__/v5/base/observables.js
index 2b88cb003..08c6b4d96 100644
--- a/packages/mobx/__tests__/v5/base/observables.js
+++ b/packages/mobx/__tests__/v5/base/observables.js
@@ -15,7 +15,7 @@ const {
isObservableProp
} = mobx
const utils = require("../../v5/utils/test-utils")
-const { MAX_SPLICE_SIZE } = require("../../../src/internal")
+const { MAX_SPLICE_SIZE, getGlobalState } = require("../../../src/internal")
const voidObserver = function () {}
@@ -2360,3 +2360,30 @@ describe("`requiresObservable` takes precedence over global `reactionRequiresObs
expect(consoleWarnSpy).not.toHaveBeenCalled()
})
})
+
+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++
+ })
+
+ prevStateVersion = getGlobalState().stateVersion
+ o.x++
+ expect(o.x).toBe(5)
+ expect(prevStateVersion).not.toBe(getGlobalState().stateVersion)
+})
diff --git a/packages/mobx/src/core/atom.ts b/packages/mobx/src/core/atom.ts
index c62cbd703..beff74426 100644
--- a/packages/mobx/src/core/atom.ts
+++ b/packages/mobx/src/core/atom.ts
@@ -11,7 +11,8 @@ import {
propagateChanged,
reportObserved,
startBatch,
- Lambda
+ Lambda,
+ globalState
} from "../internal"
export const $mobx = Symbol("mobx administration")
@@ -66,6 +67,12 @@ export class Atom implements IAtom {
public reportChanged() {
startBatch()
propagateChanged(this)
+ // 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
endBatch()
}
diff --git a/packages/mobx/src/core/globalstate.ts b/packages/mobx/src/core/globalstate.ts
index 35de9dfea..e818775ee 100644
--- a/packages/mobx/src/core/globalstate.ts
+++ b/packages/mobx/src/core/globalstate.ts
@@ -150,6 +150,11 @@ 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/yarn.lock b/yarn.lock
index 34cd4b4b5..8cb00e84c 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -11850,12 +11850,12 @@ randomfill@^1.0.3:
safe-buffer "^5.1.0"
react-dom@^18.0.0:
- version "18.0.0"
- resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.0.0.tgz#26b88534f8f1dbb80853e1eabe752f24100d8023"
- integrity sha512-XqX7uzmFo0pUceWFCt7Gff6IyIMzFUn7QMZrbrQfGxtaxXZIcGQzoNpRLE3fQLnS4XzLLPMZX2T9TRcSrasicw==
+ version "18.2.0"
+ resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.2.0.tgz#22aaf38708db2674ed9ada224ca4aa708d821e3d"
+ integrity sha512-6IMTriUmvsjHUjNtEDudZfuDQUoWXVxKHhlEGSk81n4YFS+r/Kl99wXiwlVXtPBtJenozv2P+hxDsw9eA7Xo6g==
dependencies:
loose-envify "^1.1.0"
- scheduler "^0.21.0"
+ scheduler "^0.23.0"
react-error-boundary@^3.1.0:
version "3.1.4"
@@ -11897,9 +11897,9 @@ react-test-renderer@^18.0.0:
scheduler "^0.21.0"
react@^18.0.0:
- version "18.0.0"
- resolved "https://registry.yarnpkg.com/react/-/react-18.0.0.tgz#b468736d1f4a5891f38585ba8e8fb29f91c3cb96"
- integrity sha512-x+VL6wbT4JRVPm7EGxXhZ8w8LTROaxPXOqhlGyVSrv0sB1jkyFGgXxJ8LVoPRLvPR6/CIZGFmfzqUa2NYeMr2A==
+ version "18.2.0"
+ resolved "https://registry.yarnpkg.com/react/-/react-18.2.0.tgz#555bd98592883255fa00de14f1151a917b5d77d5"
+ integrity sha512-/3IjMdb2L9QbBdWiW5e3P2/npwMBaU9mHCSCUzNln0ZCYbcfTsGbTJrU/kGemdH2IWmB2ioZ+zkxtmq6g09fGQ==
dependencies:
loose-envify "^1.1.0"
@@ -12571,6 +12571,13 @@ scheduler@^0.21.0:
dependencies:
loose-envify "^1.1.0"
+scheduler@^0.23.0:
+ version "0.23.0"
+ resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.23.0.tgz#ba8041afc3d30eb206a487b6b384002e4e61fdfe"
+ integrity sha512-CtuThmgHNg7zIZWAXi3AsyIzA3n4xx7aNyjwC2VJldO2LMVDhFK+63xGqq6CsJH4rTAt6/M+N4GhZiDYPx9eUw==
+ dependencies:
+ loose-envify "^1.1.0"
+
schema-utils@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/schema-utils/-/schema-utils-1.0.0.tgz#0b79a93204d7b600d4b2850d1f66c2a34951c770"
@@ -14078,6 +14085,11 @@ url@^0.11.0:
punycode "1.3.2"
querystring "0.2.0"
+use-sync-external-store@^1.2.0:
+ version "1.2.0"
+ resolved "https://registry.yarnpkg.com/use-sync-external-store/-/use-sync-external-store-1.2.0.tgz#7dbefd6ef3fe4e767a0cf5d7287aacfb5846928a"
+ integrity sha512-eEgnFxGQ1Ife9bzYs6VLi8/4X6CObHMw9Qr9tPY43iKwsPw8xE8+EFsf/2cFZ5S3esXgpWgtSCtLNS41F+sKPA==
+
use@^3.1.0:
version "3.1.1"
resolved "https://registry.yarnpkg.com/use/-/use-3.1.1.tgz#d50c8cac79a19fbc20f2911f56eb973f4e10070f"