Refactor QueryManager to make better use of observables and enforce fetchPolicy more reliably

[benjamn]

Fixes #5991, #5790, and multiple other related issues (full list TBD).

This is a large pull request, representing about a month and a half of work, but the central idea is relatively simple: the most important job of the QueryManager is to tirelessly enforce the requested FetchPolicy (cache-first, cache-and-network, network-only, etc.) for each query that it manages. Previously, the QueryManager sometimes failed to fulfill this mission, because the logic for interpreting FetchPolicy values was scattered across the codebase in an ad hoc manner, rather than centralized in one place.

For example, the default cache-first fetch policy should reliably trigger a network request if the relevant cache data disappears or becomes incomplete (#5991), but the code that detected that incompleteness (queryListenerForObserver) was not responsible for triggering network requests, and giving it the ability to call fetchQuery would have required yet another one-off, decentralized interpretation of options.fetchPolicy.

The scattering of FetchPolicy logic made it very difficult for anyone (including me) to understand how each fetch policy works at a glance, adding risk to any changes that involved fetch policies.

Thanks to the changes in this PR, the logic for interpreting fetch policies has been centralized into a single switch statement, which I think reads almost as clearly as pseudocode:

switch (fetchPolicy) {
  default: case "cache-first": {
    const diff = readCache();

    if (diff.complete) {
      return [
        resultsFromCache(diff, queryInfo.markReady()),
      ];
    }

    if (returnPartialData) {
      return [
        resultsFromCache(diff),
        resultsFromLink(true),
      ];
    }

    return [
      resultsFromLink(true),
    ];
  }

  case "cache-and-network": {
    const diff = readCache();

    if (diff.complete || returnPartialData) {
      return [
        resultsFromCache(diff),
        resultsFromLink(true),
      ];
    }

    return [
      resultsFromLink(true),
    ];
  }

  case "cache-only":
    return [
      resultsFromCache(readCache(), queryInfo.markReady()),
    ];

  case "network-only":
    return [resultsFromLink(true)];

  case "no-cache":
    return [resultsFromLink(false)];

  case "standby":
    return [];
}

The arrays returned by this code contain zero, one, or two observables, derived from the cache and/or the network link. The observables are concatenated together to form a single observable (specifically, a Concast observable) whose results are then consumed by the ObservableQuery (with any adjacent duplicate results skipped, as usual). At any point in time, most often when cache data has changed, or the application specifically requested a refetch, the ObservableQuery can reobserve the cache and/or the network by invoking this same code again. In other words, this switch statement is now the beating heart of the client codebase, and no data flows through the client without honoring the requested FetchPolicy.

This new system (fetchQueryObservable) is much more Observable-oriented than the previous fetchQuery method was, and I see this shift as a huge improvement. One of the reasons the QueryManager was so complicated was that the old fetchQuery method could return only a single Promise<ApolloQueryResult<T>>, even though there was often more than one result to deliver, necessitating a side-delivery mechanism (broadcastQueries). By allowing fetchQueryObservable to return multiple results using an Observable, we no longer have to worry about delivering different results using different mechanisms, since all results flow through a straightforward Observable pipeline.

Since this is such a deep refactoring, I found it impossible to keep existing tests passing during the initial exploration. Once I was happy with the shape of the new system, I resumed running and debugging the tests, and rebased any necessary code changes back into my previous commits. There was never any hope of leaving all existing tests unchanged, but I think/hope the test changes I made (see commits towards the end of this PR) are justifiable. The whole point of this refactoring was to make Apollo Client do a better job of fetching data, and "better" sometimes means "different." In any case, if you see something I changed in the tests that worries you, please leave comments/questions in-line.

The most substantial commit is Replace fetch{Query,Request} with new fetchQueryObservable method (1ac5d8d). I regret that so much ended up in that commit, but it's also a commit whose parts don't work until you put them all together, so I'm not going to waste time trying to split it up at this point. For what it's worth, everything that comes before that commit is "safe" in the sense of keeping all existing tests passing.

I will leave some comments below to draw attention to any interesting/controversial changes.

[3nvi]

Tried npm-installing it through the PR branch and building it locally, but had some issues. Is there a cache we can get a preview version on NPM to test it (whenever it's ready/stable)?

[benjamn]

@3nvi I've had some luck with running npm pack in the apollo-client/dist directory after a successful npm run build, and then doing

npm install ../path/to/apollo-client/dist/apollo-client-3.0.0-beta.*.tgz

to install that packaged .tgz file locally in other projects, but I will try to get a preview release published to npm today/soon!

[benjamn]

Ok, I can reproduce some of the tests failures locally if I use Node 10, which is what our CircleCI job uses. Locally I'd been using Node 12.16.1. No theory yet on what the operative difference between the Node versions might be. Fixed below!

[darkbasic]

@benjamn I suggest using multiple node versions on CI, one per major release.