diff --git a/src/useObserver.ts b/src/useObserver.ts index cc53d45f..cfeaa648 100644 --- a/src/useObserver.ts +++ b/src/useObserver.ts @@ -1,8 +1,8 @@ import { Reaction } from "mobx" -import { useDebugValue, useRef } from "react" +import { useDebugValue, useEffect, useRef } from "react" import { printDebugValue } from "./printDebugValue" import { isUsingStaticRendering } from "./staticRendering" -import { useForceUpdate, useUnmount } from "./utils" +import { useForceUpdate } from "./utils" export type ForceUpdateHook = () => () => void @@ -25,17 +25,25 @@ export function useObserver( const forceUpdate = wantedForceUpdateHook() const reaction = useRef(null) + const committed = useRef(false) + if (!reaction.current) { + // First render for this component. Not yet committed. reaction.current = new Reaction(`observer(${baseComponentName})`, () => { - forceUpdate() + // Observable has changed. Only force an update if we've definitely + // been committed. + if (committed.current) { + forceUpdate() + } }) } useDebugValue(reaction, printDebugValue) - useUnmount(() => { - reaction.current!.dispose() - }) + useEffect(() => { + committed.current = true + return () => reaction.current!.dispose() + }, []) // render the original component, but have the // reaction track the observables, so that rendering diff --git a/src/utils.ts b/src/utils.ts index de47b8dc..3f592878 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,10 +1,4 @@ -import { useCallback, useEffect, useState } from "react" - -const EMPTY_ARRAY: any[] = [] - -export function useUnmount(fn: () => void) { - useEffect(() => fn, EMPTY_ARRAY) -} +import { useCallback, useState } from "react" export function useForceUpdate() { const [, setTick] = useState(0) diff --git a/test/useObserver.test.tsx b/test/useObserver.test.tsx new file mode 100644 index 00000000..2e8be20c --- /dev/null +++ b/test/useObserver.test.tsx @@ -0,0 +1,42 @@ +import mockConsole from "jest-mock-console" +import * as mobx from "mobx" +import * as React from "react" +import { act, cleanup, render } from "react-testing-library" + +import { useObserver } from "../src" + +afterEach(cleanup) + +test("uncommitted observing components should not attempt state changes", () => { + const store = mobx.observable({ count: 0 }) + + const TestComponent = () => useObserver(() =>
{store.count}
) + + // Render our observing component wrapped in StrictMode + const rendering = render( + + + + ) + + // That will have caused our component to have been rendered + // more than once, but when we unmount it'll only unmount once. + rendering.unmount() + + // Trigger a change to the observable. If the reactions were + // not disposed correctly, we'll see some console errors from + // React StrictMode because we're calling state mutators to + // trigger an update. + const restoreConsole = mockConsole() + try { + act(() => { + store.count++ + }) + + // Check to see if any console errors were reported. + // tslint:disable-next-line: no-console + expect(console.error).not.toHaveBeenCalled() + } finally { + restoreConsole() + } +})