Skip to content

Commit 7516b23

Browse files
authored
Update useOnMountEffect to pass isMountedRef to callback (#2413)
## Summary: Without this, consumers of useOnMountEffect have to keep track of whether the hosting component is mounted or not when running async code within the effect. Issue: None ## Test plan: - let CI run automated checks Author: kevinb-khan Reviewers: marcysutton Required Reviewers: Approved By: marcysutton Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ⏭️ dependabot Pull Request URL: #2413
1 parent 703d793 commit 7516b23

3 files changed

Lines changed: 131 additions & 5 deletions

File tree

.changeset/tall-shoes-enjoy.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@khanacademy/wonder-blocks-core": minor
3+
---
4+
5+
Update useOnMountEffect to pass isMountedRef to callback

packages/wonder-blocks-core/src/hooks/__tests__/use-on-mount-effect.test.ts

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import {renderHook} from "@testing-library/react";
1+
import * as React from "react";
2+
import {renderHook, waitFor} from "@testing-library/react";
23

34
import {useOnMountEffect} from "../use-on-mount-effect";
45

@@ -27,4 +28,90 @@ describe("#useOnMountEffect", () => {
2728
// Assert
2829
expect(cleanup).toHaveBeenCalled();
2930
});
31+
32+
it("should pass {current: true} as isMountedRef on initial render", () => {
33+
// Arrange
34+
const callback = jest.fn();
35+
36+
// Act
37+
renderHook(() => useOnMountEffect(callback));
38+
39+
// Assert
40+
expect(callback).toHaveBeenCalledWith({current: true});
41+
});
42+
43+
it("should pass {current: false} as isMountedRef after being unmounted", () => {
44+
// Arrange
45+
const callback = jest.fn();
46+
47+
// Act
48+
const {unmount} = renderHook(() => useOnMountEffect(callback));
49+
unmount();
50+
51+
// Assert
52+
expect(callback).toHaveBeenCalledWith({current: false});
53+
});
54+
55+
describe("async", () => {
56+
it("should pass {current: true} while mounted", async () => {
57+
// Arrange
58+
const wait = async (duration: number) => {
59+
return new Promise((resolve) => {
60+
setTimeout(resolve, duration);
61+
});
62+
};
63+
let foo = false;
64+
const callback = (
65+
isMountedRef: React.MutableRefObject<boolean>,
66+
) => {
67+
const action = async () => {
68+
await wait(100);
69+
if (isMountedRef.current) {
70+
foo = true;
71+
}
72+
};
73+
74+
action();
75+
};
76+
77+
// Act
78+
renderHook(() => useOnMountEffect(callback));
79+
80+
// Assert
81+
await waitFor(() => {
82+
expect(foo).toEqual(true);
83+
});
84+
});
85+
86+
it("should pass {current: false} after being unmounted", async () => {
87+
// Arrange
88+
const wait = async (duration: number) => {
89+
return new Promise((resolve) => {
90+
setTimeout(resolve, duration);
91+
});
92+
};
93+
let foo = false;
94+
const callback = (
95+
isMountedRef: React.MutableRefObject<boolean>,
96+
) => {
97+
const action = async () => {
98+
await wait(100);
99+
if (isMountedRef.current) {
100+
foo = true;
101+
}
102+
};
103+
104+
action();
105+
};
106+
107+
// Act
108+
const {unmount} = renderHook(() => useOnMountEffect(callback));
109+
unmount();
110+
111+
// Assert
112+
await waitFor(() => {
113+
expect(foo).toEqual(false);
114+
});
115+
});
116+
});
30117
});

packages/wonder-blocks-core/src/hooks/use-on-mount-effect.ts

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import * as React from "react";
55
*
66
* If the callback returns a cleanup function, it will be called when the component is unmounted.
77
*
8-
* @param {() => (void | (() => void))} callback function that forces the component to update.
8+
* @param {(isMountedRef: React.MutableRefObject<boolean>) => void | (() => void)} callback function that forces the component to update.
99
*
1010
* The following code snippets are equivalent
1111
* ```
@@ -27,8 +27,42 @@ import * as React from "react";
2727
* }, []);
2828
*
2929
* If you only need to do something on mount, don't return a cleanup function from `callback`.
30+
*
31+
* If your callback is async, use the `isMountedRef` ref that's passed to the callback to ensure
32+
* that the component using `useOnMountEffect` hasn't been unmounted, e.g.
33+
*
34+
* ```
35+
* const MyComponent = () => {
36+
* const [foo, setFoo] = React.useState("");
37+
* useOnMountEffect((isMountedRef) => {
38+
* const action = async () => {
39+
* const res = await fetch("/foo");
40+
* const text = res.text();
41+
* if (isMountedRef.current) {
42+
* setFoo(text);
43+
* }
44+
* }
45+
*
46+
* action();
47+
* });
48+
*
49+
* return foo !== "" ? <h1>Loading...</h1> : <h1>{foo}</h1>;
50+
* }
3051
*/
31-
export const useOnMountEffect = (callback: () => void | (() => void)): void => {
32-
// eslint-disable-next-line react-hooks/exhaustive-deps
33-
React.useEffect(callback, []);
52+
export const useOnMountEffect = (
53+
callback: (
54+
isMountedRef: React.MutableRefObject<boolean>,
55+
) => void | (() => void),
56+
): void => {
57+
const isMountedRef = React.useRef(true);
58+
59+
React.useEffect(() => {
60+
const cleanup = callback(isMountedRef);
61+
62+
return () => {
63+
cleanup?.();
64+
isMountedRef.current = false;
65+
};
66+
// eslint-disable-next-line react-hooks/exhaustive-deps
67+
}, []);
3468
};

0 commit comments

Comments
 (0)