Make wdk-client service cache invalidation lazy to improve webpage load time when cache is primed

[dmfalke]

Partially addresses #987

This PR attempts to further reduce website load time (with a primed cache).

Previously, we would always make a request to the WDK project service endpoint, and compare startup time with the value stored in indexeddb. If the values differ, indexeddb is cleared, and all requests proceed as normal, repopulated indexeddb. This request would block other requests, which is unfortunate when the startup time has not changed.

In this PR, we are being a less eager and relying on the header mismatch response from requests, to detect an out-of-date set of values in indexeddb. In that scenario, the store is cleared, and the page is reloaded. If the user has been on the page for a while, they will be alerted of the reload, otherwise it will happen silently.

With this change, page loads are faster when the backend hasn't been started or rebuilt since the user was last on the website; otherwise, the "slient" reload can look a little glitchy, as there will be a flash of content on the screen before the reload happens. I haven't thought of a good way to avoid this, and not sure there is.

In the future, I would like to be able to refresh the out-of-date cache items in the background, without reloading the page. This could potentially be done by having WdkService emit an event that redux stores listen to and refresh data, as necessary. We can also explore using react-query (see #1068 for more details).

[dmfalke]

I think this is in a good state. I will wait until the release is done to open this for review.

[dmfalke]

FYI, I made #1068, as a potential "next phase" for this work. For instance, we can configure TanStack Query to use cached resources, but still update them in the background.

[bobular]

Hi @dmfalke - I had another careful read-through/understanding of the code and also tested a local plasmo genomics-site and the eda react dev server and it behaves as intended. I get a 'silent' reload if switching my genomics-site back end between plasmo and VB (because they have different startupTimes). I got the alert() reload once with the react dev server, not exactly sure how to reproduce that without running my own back end. Oh yes, I can do it by leaving the local genomics-site tab 'A' open while restarting it with a different back end (opens tab 'B' in my browser), then going back to tab 'A' and interacting with it - causes the alert() -> OK refresh.

Comparing qa.plasmo with 'local' plasmo genomics-site, the timings are definitely improved

qa: blocking /service call starts at 1.6s and takes 0.6s, first 'real' back end request starts around 2.5s

this branch: no blocking /service call, first 'real' back end request starts at around 1.0s

So that has shaved off around 1.5s from iframe page loads!

I added some debugging to understand the exact meaning of _firstRealRequestMade

    return fetchWithRetry(1, isBaseUrl ? url : serviceUrl + url, {
      headers,
      method: method.toUpperCase(),
      body: body,
      credentials: 'include',
    })
      .then((response) => {
	console.log(`_firstRealRequestMade is ${_firstRealRequestMade} for url ${url}`);
        let firstRealRequestMade = _firstRealRequestMade;
        _firstRealRequestMade = true;
        if (_isInvalidating) {
          return pendingPromise as Promise<T>;
        }

        if (response.ok) {

My initial thoughts were that the first "real" request was the request that wasn't for the /wdk-service endpoint to get the service info including startTime.

But my logging shows that sometimes /site-messages comes first. I'm not sure if that's all part of the plan. In any case, comments in the code explaining what "real" means would be useful, I think?

[dmfalke]

@bobular thanks for the thorough review. I have to agree that _firstRealRequestMade is terribly named. It should be something like _hasRequestBeenMade, or something like that. I will also document how it is used.

Also, there is a race condition with where it is currently being set to true. It should be done outside the promise .then callback. I will fix that, too.

[dmfalke]

@bobular I made some minor changes, including adding some comments and replacing a variable (_initialCheck) with the lodash function once.

[dmfalke]

Also, there is a race condition with where it is currently being set to true. It should be done outside the promise .then callback. I will fix that, too.

It turns out that this isn't really a race condition and can stay where it is. With the code the way it is, the first request to complete, regardless of when it started, will set the value to true. This is fine, because _hasRequestBeenMade is only read when handling responses, thus no race condition.

[bobular]

Crystal clear!