dai-shi/react-hooks-worker

Stale data returning when inputs changed

doelgonzo opened this issue · 4 comments

I believe I found a bug. When I pass in a value to a useWorker that relies on a param, the result appears to be cached. It's hard to explain... for the following code:

const {result} = useWorker(createExpensiveFetch, someParam)
console.log(someParam, result)

It produces console logs like this...

123, { a: 'Hello World', b: 123 },

Then on change of someParam to be 124, I see:

124, { a: 'Hello World', b: 123 },
124, { a: 'Foo Bar', b: 124 },

Basically, I have a hook that takes a prop that can change, and it takes tome to process the worker data (think a data fetch)... but when the param changes, the result/error values seem to be the ones corresponding to the previous param. I've gotten around it by making sure the value of result.b matches someParam, but this seems like a bug?

While I understand this is somewhat unexpected, this is intentional behavior.
The background is we allow async generators, which means it can get multiple results for one input. So, having loading isn't technically possible. (Maybe "pending for the first result" might be possible. But, another pattern would be the worker waits for two inputs to return one result..)

So, at this moment, what you did is something I expected as a usage, and I'd do the same.

That said, I'm open for discussions.
FYW, in https://github.com/dai-shi/react-suspense-worker, I took the RPC approach for simplicity.

Perhaps a simple flag in the response would do the trick. For example, how I got around it now is that I pass in a $inputKey value that is related to my input data. For example:

const [key, setKey] = useState(123)
const workerInput = useMemo(() => ({
  $inputKey: key,
  ...otherParams
}, [key, otherParams])

const {result, error} = useWorker(<createWorker>, workerInput)

I make sure the output of the worker returns the same $inputKey in the resulting object. Then I can do this:

const isOutputStale = useMemo(() => workerInput?.$inputKey !== result?.$inputKey, [workerInput, result])

However, it's a pattern I will have to follow on every single use of this hook. If we could somehow use the dependency to flag it as stale because we're returning an old result, it would skip this step pretty easily. Basically adding a isStale flag to the return value of useWorker. This could leverage the commonplace usePrevious hook to determine that the input doesn't match the previous one, and flag it as non-stale when the window.onMessage receives a message associated to that input... Not sure it would work for the async version of it, but it could at least help with that piece of it.

I just suspect that with the hook pattern, it will seem unexpected to be receiving stale results with no indication of them being stale.

If this is not an option, this behavior should probably be described in the Readme

Yeah, the notion of "stale" only works for RPC style (one request / one response).
While I understand that the RPC style is majority, this library is trying to be more than that.
So, for now, it's intentional and we should note it in README with a use case recipe for isStale.