search

brillout / react-streaming

React Streaming. Full-fledged & Easy.
MIT License
216 stars 14 forks source link

useAsync/useSSRData with dependencies #10

Closed jaybe78 closed 2 years ago

jaybe78 commented 2 years ago

That PR allows to provide dependencies to useAsync/useSSRData so that the promise is re-executed whenever there's a change.

brillout commented 2 years ago

Did some refactoring, see commits.

jaybe78 commented 2 years ago

Did some refactoring, see commits.

  • I found a bug while doing so, see fix commit.
  • We should handle the case deps changed during suspense.
  • We should add tests, this is error prone.

looks good !

brillout commented 2 years ago

Up for tackling the last two points?

jaybe78 commented 2 years ago

Did some refactoring, see commits.

  • I found a bug while doing so, see fix commit.
  • We should handle the case deps changed during suspense.
  • We should add tests, this is error prone.

looks good !

Yes I can.

1) Regarding the deps changing during suspense, it seems to me like an unlikely scenario. It would involve that while async callback has been called and is in pending state for a while, the user updates the deps client side for example, right ?

But, if deps change during suspense, a new callback would still be called again and data updated ? We would have 2 async calls in pending state so I guess we could find a way to cancel the first promise before executing the new one ?

2) Regarding the unit test, I'm thinking I could spy on useSSrData's callback. Basically making sure it is called or not called in the different potential scenario

brillout commented 2 years ago

My idea is to just ignore the first promise. Currently the first suspense promise discards the second call even with a different deps which shouldn't happen. It should only discard if deps is the same.

jaybe78 commented 2 years ago

My idea is to just ignore the first promise. Currently the first suspense promise discards the second call even with a different deps which shouldn't happen. It should only discard if deps is the same.

ok going to look into that. cheers

brillout commented 2 years ago

Hi @jaybe78, any news on this? 👀

brillout commented 2 years ago

Released in 0.3.0. Note the slightly different interface (use key instead of deps), see README. Should be functionally equivalent.