Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mobx-react-lit): dispose reactions right after render in StrictMode and Suspense #3777

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
134 changes: 134 additions & 0 deletions packages/mobx-react-lite/__tests__/strictAndConcurrentMode.test.tsx
Expand Up @@ -2,6 +2,7 @@ import { act, cleanup, render } from "@testing-library/react"
import mockConsole from "jest-mock-console"
import * as mobx from "mobx"
import * as React from "react"
import { ErrorBoundary } from "./utils/ErrorBoundary"

import { useObserver } from "../src/useObserver"

Expand Down Expand Up @@ -65,3 +66,136 @@ test(`observable changes before first commit are not lost`, async () => {

expect(rendering.baseElement.textContent).toBe("changed")
})

test("suspended components should not leak observations", () => {
const o = mobx.observable({ x: 0, promise: null as Promise<void> | null })
const Cmp = () =>
useObserver(() => {
o.x as any // establish dependency

if (o.promise) {
throw o.promise
}

return <>{o.x}</>
})

const observed = jest.fn()
const unobserved = jest.fn()
mobx.onBecomeObserved(o, "x", observed)
mobx.onBecomeUnobserved(o, "x", unobserved)

jest.useFakeTimers()
const { container, unmount } = render(
<React.Suspense fallback={"loading..."}>
<Cmp />
</React.Suspense>
)

jest.runAllTimers()
expect(container).toHaveTextContent("0")
expect(observed).toBeCalledTimes(1)

let resolve: () => void
act(() => {
o.promise = new Promise(r => (resolve = r))
})

jest.runAllTimers()
expect(container).toHaveTextContent("loading...")
expect(observed).toBeCalledTimes(1)
expect(unobserved).toBeCalledTimes(0)

act(() => {
o.promise = null
resolve!()
})

jest.runAllTimers()
expect(container).toHaveTextContent(o.x + "")

// ensure that we using same reaction and component state
expect(observed).toBeCalledTimes(1)
expect(unobserved).toBeCalledTimes(0)

act(() => {
o.x++
})

jest.runAllTimers()
expect(container).toHaveTextContent(o.x + "")

unmount()

jest.runAllTimers()
expect(observed).toBeCalledTimes(1)
expect(unobserved).toBeCalledTimes(1)
jest.useRealTimers()
})

test("uncommitted components should not leak observations", async () => {
const store = mobx.observable({ count1: 0, count2: 0 })

// Track whether counts are observed
let count1IsObserved = false
let count2IsObserved = false
mobx.onBecomeObserved(store, "count1", () => (count1IsObserved = true))
mobx.onBecomeUnobserved(store, "count1", () => (count1IsObserved = false))
mobx.onBecomeObserved(store, "count2", () => (count2IsObserved = true))
mobx.onBecomeUnobserved(store, "count2", () => (count2IsObserved = false))

const TestComponent1 = () => useObserver(() => <div>{store.count1}</div>)
const TestComponent2 = () => useObserver(() => <div>{store.count2}</div>)

jest.useFakeTimers()
// Render, then remove only #2
const rendering = render(
<React.StrictMode>
<TestComponent1 />
<TestComponent2 />
</React.StrictMode>
)
rendering.rerender(
<React.StrictMode>
<TestComponent1 />
</React.StrictMode>
)

jest.runAllTimers()
// count1 should still be being observed by Component1,
// but count2 should have had its reaction cleaned up.
expect(count1IsObserved).toBeTruthy()
expect(count2IsObserved).toBeFalsy()

jest.useRealTimers()
})

test("abandoned components should not leak observations", async () => {
const store = mobx.observable({ count: 0 })

let countIsObserved = false
mobx.onBecomeObserved(store, "count", () => (countIsObserved = true))
mobx.onBecomeUnobserved(store, "count", () => (countIsObserved = false))

const TestComponent = () =>
useObserver(() => {
store.count // establish dependency
throw new Error("not rendered")
})

jest.useFakeTimers()

render(
<ErrorBoundary fallback="error">
<TestComponent />
</ErrorBoundary>
)

expect(countIsObserved).toBeTruthy()

jest.runAllTimers()

expect(countIsObserved).toBeFalsy()

jest.useRealTimers()
})

This file was deleted.

This file was deleted.