Skip to content

Commit

Permalink
add shouldReportChanged
Browse files Browse the repository at this point in the history
  • Loading branch information
jzhan-canva committed Jun 4, 2024
1 parent 6ff48f3 commit 3a8c951
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 10 deletions.
8 changes: 5 additions & 3 deletions packages/mobx-react/__tests__/observer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {
computed,
observable,
transaction,
makeObservable
makeObservable,
runInAction
} from "mobx"
import { withConsole } from "./utils/withConsole"
import { shallowEqual } from "../src/utils/utils"
Expand Down Expand Up @@ -502,14 +503,14 @@ describe("should render component even if setState called with exactly the same
expect(renderCount).toBe(2)
})

test("after click twice renderCount === 3", () => {
test("after click twice renderCount === 2", () => {
const { container } = render(<Comp />)
const clickableDiv = container.querySelector("#clickableDiv") as HTMLElement

act(() => clickableDiv.click())
act(() => clickableDiv.click())

expect(renderCount).toBe(3)
expect(renderCount).toBe(2)
})
})

Expand Down Expand Up @@ -1129,6 +1130,7 @@ test("Class observer can react to changes made before mount #3730", () => {

@observer
class Child extends React.Component {
@action
componentDidMount(): void {
o.set(1)
}
Expand Down
39 changes: 32 additions & 7 deletions packages/mobx-react/src/observerClass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ type ObserverAdministration = {
// forceUpdate sets this.props.
// This flag is used to avoid the loop.
isUpdating: boolean
changedVariables: WeakSet<any>
unchangedVariables: WeakSet<any>
}

function getAdministration(component: Component): ObserverAdministration {
Expand All @@ -51,7 +53,9 @@ function getAdministration(component: Component): ObserverAdministration {
propsAtom: createAtom("props"),
stateAtom: createAtom("state"),
contextAtom: createAtom("context"),
isUpdating: false
isUpdating: false,
changedVariables: new WeakSet(),
unchangedVariables: new WeakSet()
})
}

Expand Down Expand Up @@ -259,15 +263,26 @@ function observerSCU(nextProps: ClassAttributes<any>, nextState: any): boolean {
"[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."
)
}
// update on any state changes (as is the default)
if (this.state !== nextState) {
return true
}
// update if props are shallowly not equal, inspired by PureRenderMixin
// we could return just 'false' here, and avoid the `skipRender` checks etc
// however, it is nicer if lifecycle events are triggered like usually,
// so we return true here if props are shallowly modified.
return !shallowEqual(this.props, nextProps)
const propsChanged = !shallowEqual(this.props, nextProps)
const stateChanged = !shallowEqual(this.state, nextState)
const admin = getAdministration(this)
const shouldUpdate = propsChanged || stateChanged

if (propsChanged) {
nextProps && admin.changedVariables.add(nextProps)
} else {
nextProps && admin.unchangedVariables.add(nextProps)
}
if (stateChanged) {
nextState && admin.changedVariables.add(nextState)
} else {
nextState && admin.unchangedVariables.add(nextState)
}
return shouldUpdate
}

function createObservablePropDescriptor(key: "props" | "state" | "context") {
Expand All @@ -290,7 +305,7 @@ function createObservablePropDescriptor(key: "props" | "state" | "context") {
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)) {
if (!admin.isUpdating && shouldReportChanged(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,
Expand All @@ -305,6 +320,16 @@ function createObservablePropDescriptor(key: "props" | "state" | "context") {
}
}

function shouldReportChanged(admin: ObserverAdministration, key: string, value: any) {
if (admin.changedVariables.has(value)) {
return true
} else if (admin.unchangedVariables.has(value)) {
return false
} else {
return !shallowEqual(admin[key], value)
}
}

const observablePropsDescriptor = createObservablePropDescriptor("props")
const observableStateDescriptor = createObservablePropDescriptor("state")
const observableContextDescriptor = createObservablePropDescriptor("context")

0 comments on commit 3a8c951

Please sign in to comment.