Make sure 'run' always passes the latest props/options (#71)

* Make sure 'run' always passes the latest props/options, regardless of memoization. Fixes #69. * Tests must be compatible with React v16.3.
async-library · Aug 5, 2019 · 0da6eb8 · 0da6eb8
1 parent f22acd0
commit 0da6eb8
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 5 deletions.
diff --git a/packages/react-async/src/specs.js b/packages/react-async/src/specs.js
@@ -397,6 +397,46 @@ export const withDeferFn = (Async, abortCtrl) => () => {
     expect(deferFn).toHaveBeenCalledWith(["go", 2], expect.objectContaining(props), abortCtrl)
   })
 
+  test("always passes the latest props", async () => {
+    const deferFn = jest.fn().mockReturnValue(resolveTo())
+    const Child = ({ count }) => (
+      <Async deferFn={deferFn} count={count}>
+        {({ run }) => (
+          <>
+            <button onClick={() => run(count)}>run</button>
+            <div data-testid="counter">{count}</div>
+          </>
+        )}
+      </Async>
+    )
+    class Parent extends React.Component {
+      constructor(props) {
+        super(props)
+        this.state = { count: 0 }
+      }
+      render() {
+        const inc = () => this.setState(state => ({ count: state.count + 1 }))
+        return (
+          <>
+            <button onClick={inc}>inc</button>
+            {this.state.count && <Child count={this.state.count} />}
+          </>
+        )
+      }
+    }
+    const { getByText, getByTestId } = render(<Parent />)
+    fireEvent.click(getByText("inc"))
+    expect(getByTestId("counter")).toHaveTextContent("1")
+    fireEvent.click(getByText("inc"))
+    expect(getByTestId("counter")).toHaveTextContent("2")
+    fireEvent.click(getByText("run"))
+    expect(deferFn).toHaveBeenCalledWith(
+      [2],
+      expect.objectContaining({ count: 2, deferFn }),
+      abortCtrl
+    )
+  })
+
   test("`reload` uses the arguments of the previous run", () => {
     let counter = 1
     const deferFn = jest.fn().mockReturnValue(resolveTo())

diff --git a/packages/react-async/src/useAsync.js b/packages/react-async/src/useAsync.js
@@ -11,7 +11,7 @@ const useAsync = (arg1, arg2) => {
   const counter = useRef(0)
   const isMounted = useRef(true)
   const lastArgs = useRef(undefined)
-  const prevOptions = useRef(undefined)
+  const lastOptions = useRef(undefined)
   const abortController = useRef({ abort: noop })
 
   const { devToolsDispatcher } = globalScope.__REACT_ASYNC__
@@ -72,7 +72,7 @@ const useAsync = (arg1, arg2) => {
     }
     const isPreInitialized = initialValue && counter.current === 0
     if (promiseFn && !isPreInitialized) {
-      return start(() => promiseFn(options, abortController.current)).then(
+      return start(() => promiseFn(lastOptions.current, abortController.current)).then(
         handleResolve(counter.current),
         handleReject(counter.current)
       )
@@ -83,7 +83,7 @@ const useAsync = (arg1, arg2) => {
   const run = (...args) => {
     if (deferFn) {
       lastArgs.current = args
-      return start(() => deferFn(args, options, abortController.current)).then(
+      return start(() => deferFn(args, lastOptions.current, abortController.current)).then(
         handleResolve(counter.current),
         handleReject(counter.current)
       )
@@ -99,15 +99,15 @@ const useAsync = (arg1, arg2) => {
 
   const { watch, watchFn } = options
   useEffect(() => {
-    if (watchFn && prevOptions.current && watchFn(options, prevOptions.current)) load()
+    if (watchFn && lastOptions.current && watchFn(options, lastOptions.current)) load()
   })
+  useEffect(() => (lastOptions.current = options) && undefined)
   useEffect(() => {
     if (counter.current) cancel()
     if (promise || promiseFn) load()
   }, [promise, promiseFn, watch])
   useEffect(() => () => (isMounted.current = false), [])
   useEffect(() => () => cancel(), [])
-  useEffect(() => (prevOptions.current = options) && undefined)
 
   useDebugValue(state, ({ status }) => `[${counter.current}] ${status}`)