ava / use-http

🐶 React hook for making isomorphic http requests
https://use-http.com
MIT License
2.31k stars 114 forks source link

Auto-Managed Suspense API - New data does not trigger a re-rendering of the component #257

Open jnterry opened 4 years ago

jnterry commented 4 years ago

Describe the bug

In 1.0.9, the suspense API with auto-managed state does not trigger a re-render of the component once an updated response is recieved. This means the rendered data is stale as compared to the requested URL.

Codesandbox

Here is a demo which allows the user to enter a domain name to query, and then makes a request to dns.google.com's JSON API in order to find its IP address: https://codesandbox.io/s/3poib?file=/src/App.js

To Reproduce Using the codesandbox linked above:

  1. Load the page - result for API query of 'google.com' is displayed
  2. Type 'bbc.com' into the input field and press enter and/or the query button any number of times
  3. In the console, we see the useFetch interceptor fire, stating new data has been recieved. However this occurs after re-rendering the component, and thus the data previously fetched for 'google.com' is still displayed on the page
  4. Now type 'example.com' into the input field and press enter/query
  5. The compent re-renders, and thus we now see the API response for 'bbc.com'. After re-rendering, the useFetch interceptor fires, and we see in the console that new data has been retrieved - but again, this does not trigger a re-render of the component

Expected behavior

After useFetch recieves new data (in response to its dependency array changing) the component which make the call to the useFetch hook should be re-rendered such that the displayed data is always the most up to date version available.

The behaviour which I expect can be emulated using the suspense managed state API: https://codesandbox.io/s/wfdvs?file=/src/App.js Try the same sequence of steps as above to see the difference.

jnterry commented 4 years ago

As someone not familiar with this codebase (but having hacked around in my node_modules installation) I think swapping the order of the following lines would solve this:

forceUpdate()
const newData = await suspender.current

(https://github.com/ava/use-http/blob/master/src/useFetch.ts#L192-L193)

Note however that this does not "re-suspend" the component - IE: while waiting for the response from the second request, I don't see the <Suspense>'s fallback component being rendered which I would expect (I instead still see the content from the previous request).

So prehaps it would make sense to also reset the suspenseStatus to pending somehow (although just adding a line to do this just before https://github.com/ava/use-http/blob/master/src/useFetch.ts#L182 didn't seem to help).

I think re-suspending makes sense while a second request in progress - since the current behaviour where the old content is displayed could be achieved using react's "useTransition" (https://reactjs.org/docs/concurrent-mode-patterns.html#transitions). (Admittedly this isn't yet available in the stable react branch).

alex-cory commented 4 years ago

Will get to this as soon as possible! Thank you for pointing this out and doing some investigating!

balthazar commented 4 years ago

Also interested in this, I did a small codesandbox to reproduce the issue:

Clicking the button the first time correctly shows the Suspense state, but all subsequent clicks do not, and update the state only once the request has completed.

@alex-cory Is there something I could do to help?

sgtwilko commented 4 years ago

I agree with @jnterry :

Note however that this does not "re-suspend" the component - IE: while waiting for the response from the second request, I don't see the <Suspense>'s fallback component being rendered which I would expect (I instead still see the content from the previous request).

So prehaps it would make sense to also reset the suspenseStatus to pending somehow (although just adding a line to do this just before https://github.com/ava/use-http/blob/master/src/useFetch.ts#L182 didn't seem to help).

I think re-suspending makes sense while a second request in progress - since the current behaviour where the old content is displayed could be achieved using react's "useTransition" (https://reactjs.org/docs/concurrent-mode-patterns.html#transitions). (Admittedly this isn't yet available in the stable react branch).

The component should def "re-suspend" when a second/subsequent call is made. We spent a while trying to work out where our code wasn't working, before realising it was an issue in use-fetch.

sam17896 commented 3 years ago

is there any progress on this .. is there any work around to this ??

flying-sheep commented 2 years ago

Ah, so it’s simply a bug. I wracked my brain why this doesn’t work.

Can you at least remove the broken example from your webpage until this is fixed?