ObservableQuery.getCurrentResult: skip the cache #10631

phryneas · 2023-03-08T16:40:02Z

This is my attempt at solving #10222.

The test is taken from @jerelmiller over in #10239.

I'm not sure if this covers all possible cases (what should happen if all of this plays out on a subsequent query? Is that even possible there?), but at least it gets all the tests green.

One of the HOC tests is now rerendering one less time, but not getting any new data.

I'll add a changeset if we decide that this solution is feasible.

Checklist:

If this PR contains changes to the library itself (not necessary for e.g. docs updates), please include a changeset (see CONTRIBUTING.md)
If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
Make sure all of the significant new logic is covered by tests

…nent is unmount and remounted

if currently running the first query, started with a fetchPolicy meant to ignore cache results

changeset-bot · 2023-03-08T16:40:10Z

🦋 Changeset detected

Latest commit: ee8dda5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@apollo/client	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

phryneas · 2023-03-08T16:44:57Z

.vscode/settings.json

@@ -7,5 +7,6 @@
  "typescript.tsdk": "node_modules/typescript/lib",
  "cSpell.enableFiletypes": [
    "mdx"
-  ]
+  ],
+  "jest.jestCommandLine": "node_modules/.bin/jest --config ./config/jest.config.js --ignoreProjects 'ReactDOM 17' --runInBand",


I had to add this with some commit - the Jest VSCode plugin cannot display it when two projects run the same test twice, so I just set it to exclude the React17 tests. Also adds --runInBand for the extension, which makes tests less flaky in general.

phryneas · 2023-03-08T16:46:03Z

src/core/ObservableQuery.ts

@@ -152,6 +154,7 @@ export class ObservableQuery<
    this.queryManager = queryManager;

    // active state
+    this.waitForOwnResult = skipCacheDataFor(options.fetchPolicy);


This has to be initialized in the constructor, as getCurrentResult() will be called before the ObservableQuery is subscribed to for the first time.

phryneas · 2023-03-08T16:46:54Z

src/react/hoc/__tests__/queries/loading.test.tsx

@@ -421,7 +421,7 @@ describe('[queries] loading', () => {
      ]);
    }, { interval: 1 });
    await waitFor(() => {
-      expect(count).toBe(6);
+      expect(count).toBe(5);


This now rerenders one less time. I checked it - render 5 and 6 had identical this.props.data, so this is probably a good thing:tm:?

phryneas · 2023-03-08T17:22:27Z

(Tests are all green now, rerunning the test suite a third time. All that's red at this point is the file size.)

src/core/__tests__/ObservableQuery.ts

github-actions · 2023-05-31T13:08:32Z

size-limit report 📦

Path	Size
dist/apollo-client.min.cjs	33.74 KB (+0.23% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs"	42.48 KB (+0.2% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js"	33.54 KB (+0.23% 🔺)
import { ApolloProvider } from "dist/react/index.js"	917 B (0%)
import { useQuery } from "dist/react/index.js"	25.05 KB (+0.38% 🔺)
import { useLazyQuery } from "dist/react/index.js"	25.37 KB (+0.41% 🔺)
import { useMutation } from "dist/react/index.js"	23.6 KB (+0.4% 🔺)
import { useSubscription } from "dist/react/index.js"	2.43 KB (0%)
import { useFragment_experimental } from "dist/react/index.js"	1.84 KB (0%)

phryneas · 2023-05-31T13:18:20Z

/release:pr

github-actions · 2023-05-31T13:19:58Z

A new release has been made for this PR. You can install it with npm i @apollo/client@0.0.0-pr-10631-20230531131910.

phryneas · 2023-06-14T15:57:05Z

I'm happy with the state of this now and have just added a ton of tests. Ready for review!

phryneas · 2023-06-14T15:59:22Z

/release:pr

github-actions · 2023-06-14T16:01:22Z

A new release has been made for this PR. You can install it with npm i @apollo/client@0.0.0-pr-10631-20230614160035.

benjamn

Thanks for finally cracking this important puzzle @phryneas!

benjamn · 2023-06-21T16:02:10Z

src/core/ObservableQuery.ts

+function skipCacheDataFor(fetchPolicy?: WatchQueryFetchPolicy) {
+  return fetchPolicy === "network-only" || fetchPolicy === "no-cache" || fetchPolicy === "standby";


Might be worth a comment that when typeof fetchPolicy === "undefined", that means cache-first by default, so skipCacheDataFor should return false (as it does already).

src/core/ObservableQuery.ts

efstathiosntonas · 2023-06-21T19:03:45Z

Thanks for this guys, @benjamn do you have an ETA on this?

phryneas · 2023-06-22T08:35:48Z

@efstathiosntonas If nothing super-weird pops up, you can expect this to probably be in a patch release next week or the week after

Co-authored-by: Ben Newman <ben@apollographql.com>

src/core/ObservableQuery.ts

netlify · 2023-06-22T15:55:35Z

✅ Deploy Preview for apollo-client-docs ready!

Name	Link
🔨 Latest commit	`b360183`
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-docs/deploys/64946e0df3935f00085b8a2d
😎 Deploy Preview	https://deploy-preview-10631--apollo-client-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2023-06-22T15:59:51Z

✅ Deploy Preview for apollo-client-docs ready!

Name	Link
🔨 Latest commit	`ee8dda5`
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-docs/deploys/64947133c1ed1f000828c31c
😎 Deploy Preview	https://deploy-preview-10631--apollo-client-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

jerelmiller and others added 2 commits March 8, 2023 11:22

Write failing test to demonstrate nextFetchPolicy is used after compo…

52c433b

…nent is unmount and remounted

ObservableQuery.currentResult: skip the cache

b37bed2

if currently running the first query, started with a fetchPolicy meant to ignore cache results

phryneas requested review from jerelmiller, benjamn and alessbell March 8, 2023 16:40

phryneas mentioned this pull request Mar 8, 2023

Unexpected Behavior with fetchPolicy "network-only" and nextFetchPolicy "cache-first" In 3.7.x #10222

Closed

phryneas commented Mar 8, 2023

View reviewed changes

phryneas changed the title ~~ObservableQuery.currentResult: skip the cache~~ ObservableQuery.getCurrentResult: skip the cache Mar 9, 2023

Lenz Weber-Tronic added 2 commits March 14, 2023 11:53

format "ObservableQuery" test

0a67647

add another test for ObservableQuery behaviour

8227e66

phryneas commented Mar 14, 2023

View reviewed changes

src/core/__tests__/ObservableQuery.ts Outdated Show resolved Hide resolved

appease CI

2f0175a

phryneas force-pushed the pr/10222 branch from bfb15ab to 2f0175a Compare March 14, 2023 13:21

alessbell added the 🛫 in-progress label Apr 3, 2023

phryneas added 2 commits May 31, 2023 14:49

Merge remote-tracking branch 'origin/main' into pr/10222

0e2d94a

fixup test

66e8c83

re-add test that was lost in merge

088dc51

phryneas self-assigned this Jun 13, 2023

phryneas added 5 commits June 14, 2023 15:19

ensure updateWatch is called when skipping getDiff

0206d72

Merge remote-tracking branch 'origin/main' into pr/10222

b976e79

update bundlesize

96933af

minimize diff

71f9d89

test readability

28910a3

add more tests

aff4d65

phryneas marked this pull request as ready for review June 14, 2023 15:56

benjamn approved these changes Jun 21, 2023

View reviewed changes

Update src/core/ObservableQuery.ts

b360183

Co-authored-by: Ben Newman <ben@apollographql.com>

phryneas commented Jun 22, 2023

View reviewed changes

src/core/ObservableQuery.ts Outdated Show resolved Hide resolved

Update src/core/ObservableQuery.ts

6cf89bc

Merge branch 'main' into pr/10222

ee8dda5

phryneas merged commit b93388d into main Jun 22, 2023
11 checks passed

phryneas deleted the pr/10222 branch June 22, 2023 16:18

This was referenced Jun 22, 2023

Version Packages #11006

Merged

Version Packages (beta) #11009

Merged

github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ObservableQuery.getCurrentResult: skip the cache #10631

ObservableQuery.getCurrentResult: skip the cache #10631

phryneas commented Mar 8, 2023 •

edited

changeset-bot bot commented Mar 8, 2023 •

edited

phryneas Mar 8, 2023

phryneas Mar 8, 2023

phryneas Mar 8, 2023

phryneas commented Mar 8, 2023

github-actions bot commented May 31, 2023 •

edited

phryneas commented May 31, 2023

github-actions bot commented May 31, 2023

phryneas commented Jun 14, 2023 •

edited

phryneas commented Jun 14, 2023

github-actions bot commented Jun 14, 2023

benjamn left a comment

benjamn Jun 21, 2023

efstathiosntonas commented Jun 21, 2023

phryneas commented Jun 22, 2023

netlify bot commented Jun 22, 2023

netlify bot commented Jun 22, 2023 •

edited

		function skipCacheDataFor(fetchPolicy?: WatchQueryFetchPolicy) {
		return fetchPolicy === "network-only" \|\| fetchPolicy === "no-cache" \|\| fetchPolicy === "standby";

ObservableQuery.getCurrentResult: skip the cache #10631

ObservableQuery.getCurrentResult: skip the cache #10631

Conversation

phryneas commented Mar 8, 2023 • edited

Checklist:

changeset-bot bot commented Mar 8, 2023 • edited

🦋 Changeset detected

phryneas Mar 8, 2023

Choose a reason for hiding this comment

phryneas Mar 8, 2023

Choose a reason for hiding this comment

phryneas Mar 8, 2023

Choose a reason for hiding this comment

phryneas commented Mar 8, 2023

github-actions bot commented May 31, 2023 • edited

size-limit report 📦

phryneas commented May 31, 2023

github-actions bot commented May 31, 2023

phryneas commented Jun 14, 2023 • edited

phryneas commented Jun 14, 2023

github-actions bot commented Jun 14, 2023

benjamn left a comment

Choose a reason for hiding this comment

benjamn Jun 21, 2023

Choose a reason for hiding this comment

efstathiosntonas commented Jun 21, 2023

phryneas commented Jun 22, 2023

netlify bot commented Jun 22, 2023

✅ Deploy Preview for apollo-client-docs ready!

netlify bot commented Jun 22, 2023 • edited

✅ Deploy Preview for apollo-client-docs ready!

phryneas commented Mar 8, 2023 •

edited

changeset-bot bot commented Mar 8, 2023 •

edited

github-actions bot commented May 31, 2023 •

edited

phryneas commented Jun 14, 2023 •

edited

netlify bot commented Jun 22, 2023 •

edited