data-client icon indicating copy to clipboard operation
data-client copied to clipboard

RFC: Controller.fetchIfStale()

Open ntucker opened this issue 2 years ago • 1 comments

Motivation

  • fetch-as-render pattern results in over-fetching (non-stale data is fetched on every transition)
  • centralize more handling - less in hooks = happier world

Current world

Currently final ‘stale’ logic exists in hooks

const { data, expiryStatus, expiresAt } = controller.getResponse(endpoint, ...args, state);
const forceFetch = expiryStatus === ExpiryStatus.Invalid
const maybePromise = useMemo(() => {
    // null params mean don't do anything
    if ((Date.now() <= expiresAt && !forceFetch) || !key) return;

    return controller.fetch(endpoint, ...(args as readonly [...Parameters<E>]));
    // we need to check against serialized params, since params can change frequently
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [expiresAt, controller, key, forceFetch, state.lastReset]);

This isn’t too bad; but it does mean some logic is repeated in any implementation.

The non-lifecycle based logic can be extracted as such:

Date.now() <= expiresAt && expiryStatus !== ExpiryStatus.Invalid

Proposal

Add a new controller dispatch that will only fetch if the result is stale. If it ends up not fetching the promise will immediately resolve.

  • Only accepts sideEffect: false endpoints

Open questions

  • what should it be called
  • what should it resolve to as it might not fetch but that means the data already exists?
    • maybe denormalized form and waits until commit.
    • or perhaps the resolve value lets you know if it fetched at all
  • always returning a promise means it is unusable in hooks pre-react18. can we possibly hook up a callback in the middleware that determines whether it fetches non-async?

Dispatch resolution control

dispatch adds to action a callback. if this callback is called it returns the dispatch early.

Instead of

return next(action);
return Whatever;

And you can still continue processing the request by doing the rest of it async

// we don't wait on resolution
next(action);
return Whatever;

Doing this would also enable not having to send resolve/reject in fetch meta; but just take dispatch return value. This would be actual promise used by the NetworkManager so referential equality checks could be performed against it.

This means dispatch() now has a variable return type based on what the middlewares do. This could have weird implications for user-defined managers; so perhaps we should add type inference based on middlewares? This can be delayed tho.

ntucker avatar Jun 17 '22 16:06 ntucker

Council notes

  • Name is actually quite good
  • Migrate by doing middleware change first, then add new dispatcher
  • Still open question about best return value given this is an important use case with no access to state:

Anansi router example:

  resolveData: async (controller: Controller, match: { id: string }) => {
    if (match) {
      // don't block on comments but start fetching
      controller.fetchIfStale(CommentResource.getList, { postId: match.id });
      const post = await controller.fetchIfStale(PostResource.get, {
        id: match.id,
      });
      await Promise.allSettled([
        controller.fetchIfStale(
          UserResource.get,
          post.userId ? { id: post.userId } : (null as any),
        ),
        controller.fetchIfStale(getImage, {
          src: UserResource.fromJS({ id: post.userId }).profileImage,
        }),
      ]);
    }

ntucker avatar Jun 17 '22 19:06 ntucker

This would be really helpful for us!

Name suggestion: Controller.provide as this will provide the data (from cache if fresh, from API if stale).

gregor-mueller avatar Oct 19 '22 09:10 gregor-mueller

Thanks for the input @gregor-mueller ! What do you expect/want for the return value to be? The resolution of the promise, or the processed data from the cache?

ntucker avatar Oct 20 '22 16:10 ntucker

Thanks for the quick response! I'd expect the processed data from the cache.

gregor-mueller avatar Oct 21 '22 06:10 gregor-mueller

@gregor-mueller In case you just want the state, you can use https://resthooks.io/docs/api/Controller#getState

The rest of the functionality will be soon added

ntucker avatar Nov 20 '22 19:11 ntucker

Closing in favor of discussion: https://github.com/data-client/rest-hooks/discussions/2402

ntucker avatar Jan 31 '23 15:01 ntucker