Skip to content

Commit d3c3138

Browse files
ghengeveldghengeveld
authored
Memoize all the things. Fixes #80. (#82)
1 parent 7bbbd1f commit d3c3138

File tree

3 files changed

+119
-55
lines changed

3 files changed

+119
-55
lines changed

‎.eslintrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"plugins": ["jest", "promise", "react", "react-hooks"],
1414
"rules": {
1515
"react-hooks/rules-of-hooks": "error",
16+
"react-hooks/exhaustive-deps": "warn",
1617
"no-console": "error"
1718
},
1819
"settings": {

‎packages/react-async/src/useAsync.js

Lines changed: 85 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -21,49 +21,68 @@ const useAsync = (arg1, arg2) => {
2121
options,
2222
init
2323
)
24-
const dispatch = dispatcher
25-
? action => dispatcher(action, dispatchMiddleware(_dispatch), options)
26-
: dispatchMiddleware(_dispatch)
27-
28-
const getMeta = meta => ({ counter: counter.current, debugLabel: options.debugLabel, ...meta })
24+
const dispatch = useCallback(
25+
dispatcher
26+
? action => dispatcher(action, dispatchMiddleware(_dispatch), lastOptions.current)
27+
: dispatchMiddleware(_dispatch),
28+
[dispatcher]
29+
)
2930

30-
const setData = (data, callback = noop) => {
31-
if (isMounted.current) {
32-
dispatch({ type: actionTypes.fulfill, payload: data, meta: getMeta() })
33-
callback()
34-
}
35-
return data
36-
}
31+
const { debugLabel } = options
32+
const getMeta = useCallback(meta => ({ counter: counter.current, debugLabel, ...meta }), [
33+
debugLabel,
34+
])
35+
36+
const setData = useCallback(
37+
(data, callback = noop) => {
38+
if (isMounted.current) {
39+
dispatch({ type: actionTypes.fulfill, payload: data, meta: getMeta() })
40+
callback()
41+
}
42+
return data
43+
},
44+
[dispatch, getMeta]
45+
)
3746

38-
const setError = (error, callback = noop) => {
39-
if (isMounted.current) {
40-
dispatch({ type: actionTypes.reject, payload: error, error: true, meta: getMeta() })
41-
callback()
42-
}
43-
return error
44-
}
47+
const setError = useCallback(
48+
(error, callback = noop) => {
49+
if (isMounted.current) {
50+
dispatch({ type: actionTypes.reject, payload: error, error: true, meta: getMeta() })
51+
callback()
52+
}
53+
return error
54+
},
55+
[dispatch, getMeta]
56+
)
4557

4658
const { onResolve, onReject } = options
47-
const handleResolve = count => data =>
48-
count === counter.current && setData(data, () => onResolve && onResolve(data))
49-
const handleReject = count => error =>
50-
count === counter.current && setError(error, () => onReject && onReject(error))
51-
52-
const start = promiseFn => {
53-
if ("AbortController" in globalScope) {
54-
abortController.current.abort()
55-
abortController.current = new globalScope.AbortController()
56-
}
57-
counter.current++
58-
return new Promise((resolve, reject) => {
59-
if (!isMounted.current) return
60-
const executor = () => promiseFn().then(resolve, reject)
61-
dispatch({ type: actionTypes.start, payload: executor, meta: getMeta() })
62-
})
63-
}
59+
const handleResolve = useCallback(
60+
count => data => count === counter.current && setData(data, () => onResolve && onResolve(data)),
61+
[setData, onResolve]
62+
)
63+
const handleReject = useCallback(
64+
count => err => count === counter.current && setError(err, () => onReject && onReject(err)),
65+
[setError, onReject]
66+
)
67+
68+
const start = useCallback(
69+
promiseFn => {
70+
if ("AbortController" in globalScope) {
71+
abortController.current.abort()
72+
abortController.current = new globalScope.AbortController()
73+
}
74+
counter.current++
75+
return new Promise((resolve, reject) => {
76+
if (!isMounted.current) return
77+
const executor = () => promiseFn().then(resolve, reject)
78+
dispatch({ type: actionTypes.start, payload: executor, meta: getMeta() })
79+
})
80+
},
81+
[dispatch, getMeta]
82+
)
6483

6584
const { promise, promiseFn, initialValue } = options
66-
const load = () => {
85+
const load = useCallback(() => {
6786
if (promise) {
6887
return start(() => promise).then(
6988
handleResolve(counter.current),
@@ -77,26 +96,36 @@ const useAsync = (arg1, arg2) => {
7796
handleReject(counter.current)
7897
)
7998
}
80-
}
99+
}, [start, promise, promiseFn, initialValue, handleResolve, handleReject])
81100

82101
const { deferFn } = options
83-
const run = (...args) => {
84-
if (deferFn) {
85-
lastArgs.current = args
86-
return start(() => deferFn(args, lastOptions.current, abortController.current)).then(
87-
handleResolve(counter.current),
88-
handleReject(counter.current)
89-
)
90-
}
91-
}
102+
const run = useCallback(
103+
(...args) => {
104+
if (deferFn) {
105+
lastArgs.current = args
106+
return start(() => deferFn(args, lastOptions.current, abortController.current)).then(
107+
handleResolve(counter.current),
108+
handleReject(counter.current)
109+
)
110+
}
111+
},
112+
[start, deferFn, handleResolve, handleReject]
113+
)
114+
115+
const reload = useCallback(() => {
116+
return lastArgs.current ? run(...lastArgs.current) : load()
117+
}, [run, load])
92118

93-
const cancel = () => {
94-
options.onCancel && options.onCancel()
119+
const { onCancel } = options
120+
const cancel = useCallback(() => {
121+
onCancel && onCancel()
95122
counter.current++
96123
abortController.current.abort()
97124
isMounted.current && dispatch({ type: actionTypes.cancel, meta: getMeta() })
98-
}
125+
}, [onCancel, dispatch, getMeta])
99126

127+
/* These effects should only be triggered on changes to specific props */
128+
/* eslint-disable react-hooks/exhaustive-deps */
100129
const { watch, watchFn } = options
101130
useEffect(() => {
102131
if (watchFn && lastOptions.current && watchFn(options, lastOptions.current)) load()
@@ -108,19 +137,20 @@ const useAsync = (arg1, arg2) => {
108137
}, [promise, promiseFn, watch])
109138
useEffect(() => () => (isMounted.current = false), [])
110139
useEffect(() => () => cancel(), [])
140+
/* eslint-enable react-hooks/exhaustive-deps */
111141

112142
useDebugValue(state, ({ status }) => `[${counter.current}] ${status}`)
113143

114144
return useMemo(
115145
() => ({
116146
...state,
117-
cancel,
118147
run,
119-
reload: () => (lastArgs.current ? run(...lastArgs.current) : load()),
148+
reload,
149+
cancel,
120150
setData,
121151
setError,
122152
}),
123-
[state, deferFn, onResolve, onReject, dispatcher, reducer]
153+
[state, run, reload, cancel, setData, setError]
124154
)
125155
}
126156

@@ -138,11 +168,12 @@ const useAsyncFetch = (input, init, { defer, json, ...options } = {}) => {
138168
const doFetch = (input, init) => globalScope.fetch(input, init).then(parseResponse(accept, json))
139169
const isDefer = defer === true || ~["POST", "PUT", "PATCH", "DELETE"].indexOf(method)
140170
const fn = defer === false || !isDefer ? "promiseFn" : "deferFn"
171+
const identity = JSON.stringify({ input, init })
141172
const state = useAsync({
142173
...options,
143174
[fn]: useCallback(
144175
(_, props, ctrl) => doFetch(input, { signal: ctrl ? ctrl.signal : props.signal, ...init }),
145-
[JSON.stringify(input), JSON.stringify(init)]
176+
[identity] // eslint-disable-line react-hooks/exhaustive-deps
146177
),
147178
})
148179
useDebugValue(state, ({ counter, status }) => `[${counter}] ${status}`)

‎packages/react-async/src/useAsync.spec.js

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ describe("useAsync", () => {
5454
expect(onResolve).toHaveBeenCalledWith("done")
5555
})
5656

57-
test("calling run() will always use the latest onResolve/onReject callbacks", async () => {
57+
test("calling run() will always use the latest onResolve callback", async () => {
5858
const promiseFn = jest.fn(() => resolveTo())
5959
const deferFn = () => resolveTo()
6060
function App() {
@@ -84,6 +84,38 @@ describe("useAsync", () => {
8484
await sleep(10) // resolve deferFn
8585
expect(promiseFn).toHaveBeenLastCalledWith(expect.objectContaining({ count: 1 }), abortCtrl)
8686
})
87+
88+
test("calling run() will always use the latest onReject callback", async () => {
89+
const onReject1 = jest.fn()
90+
const onReject2 = jest.fn()
91+
const deferFn = ([count]) => Promise.reject(count)
92+
function App() {
93+
const [count, setCount] = React.useState(0)
94+
const onReject = count === 0 ? onReject1 : onReject2
95+
const { run } = useAsync({ deferFn, onReject })
96+
return <button onClick={() => run(count) && setCount(1)}>run</button>
97+
}
98+
const { getByText } = render(<App />)
99+
fireEvent.click(getByText("run"))
100+
await sleep(10) // resolve deferFn
101+
expect(onReject1).toHaveBeenCalledWith(0)
102+
fireEvent.click(getByText("run"))
103+
await sleep(10) // resolve deferFn
104+
expect(onReject2).toHaveBeenCalledWith(1)
105+
})
106+
})
107+
108+
test("does not return a new `run` function on every render", async () => {
109+
const deferFn = () => resolveTo("done")
110+
const DeleteScheduleForm = () => {
111+
const [value, setValue] = React.useState()
112+
const { run } = useAsync({ deferFn })
113+
React.useEffect(() => value && run() && undefined, [value, run])
114+
return <button onClick={() => setValue(true)}>run</button>
115+
}
116+
const component = <DeleteScheduleForm />
117+
const { getByText } = render(component)
118+
fireEvent.click(getByText("run"))
87119
})
88120

89121
describe("useFetch", () => {

0 commit comments

Comments
 (0)