RFC: Controller.fetchIfStale()

[ntucker]

Originally posted as #2053

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

💡 Need feedback on name. I don’t like how long fetchIfStale is!

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;

.e.g,

 case FETCH_TYPE:
      const promise = this.handleFetch(action, dispatch, controller);
      // This is the only case that causes any state change
      // It's important to intercept other fetches as we don't want to trigger reducers during
      // render - so we need to stop 'readonly' fetches which can be triggered in render
      if (
        action.meta.optimisticResponse !== undefined ||
        action.endpoint?.getOptimisticResponse !== undefined
      ) {
        return next(action);
      }
      return promise;

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.

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]

Add compatibility adapter to keep existing managers working with this new model; add some sort of detection so the builtin managers use the new path

[ntucker]

Controller.fetchIfStale is released in @data-client/react@0.4.0