More robust handling of local state default value when read function is defined by jerelmiller · Pull Request #12934 · apollographql/apollo-client

jerelmiller · 2025-09-22T23:55:13Z

Better detect whether the cache can handle resolving a client field and warn if neither a local resolver or read function is defined. If a read function is defined, don't set a default value of null for the field to allow for the read function to set the value. This allows default parameters to work again.

This also adds a check for no-cache fetch policies and will warn if used with a @client field since the cache is not read. This fixes an issue where using a no-cache fetch policy would read from the cache to get the cache diff to resolve the field.

changeset-bot · 2025-09-22T23:55:17Z

🦋 Changeset detected

Latest commit: 4bceff8

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

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

pkg-pr-new · 2025-09-22T23:56:49Z

npm i https://pkg.pr.new/apollographql/apollo-client/@apollo/client@12934

commit: 4bceff8

github-actions · 2025-09-22T23:57:27Z

size-limit report 📦

Path	Size
import { ApolloClient, InMemoryCache, HttpLink } from "@apollo/client" (CJS)	43.7 KB (+0.48% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "@apollo/client" (production) (CJS)	38.5 KB (-0.11% 🔽)
import { ApolloClient, InMemoryCache, HttpLink } from "@apollo/client"	33.11 KB (+0.61% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "@apollo/client" (production)	27.08 KB (+0.08% 🔺)
import { ApolloProvider } from "@apollo/client/react"	6.11 KB (+2.31% 🔺)
import { ApolloProvider } from "@apollo/client/react" (production)	1001 B (0%)
import { useQuery } from "@apollo/client/react"	7.54 KB (+1.54% 🔺)
import { useQuery } from "@apollo/client/react" (production)	2.39 KB (0%)
import { useLazyQuery } from "@apollo/client/react"	7.29 KB (+1.88% 🔺)
import { useLazyQuery } from "@apollo/client/react" (production)	2.16 KB (0%)
import { useMutation } from "@apollo/client/react"	6.66 KB (+1.98% 🔺)
import { useMutation } from "@apollo/client/react" (production)	1.52 KB (0%)
import { useSubscription } from "@apollo/client/react"	6.99 KB (+2.19% 🔺)
import { useSubscription } from "@apollo/client/react" (production)	1.82 KB (0%)
import { useSuspenseQuery } from "@apollo/client/react"	8.77 KB (+1.51% 🔺)
import { useSuspenseQuery } from "@apollo/client/react" (production)	3.67 KB (0%)
import { useBackgroundQuery } from "@apollo/client/react"	8.57 KB (+1.85% 🔺)
import { useBackgroundQuery } from "@apollo/client/react" (production)	3.44 KB (0%)
import { useLoadableQuery } from "@apollo/client/react"	8.51 KB (+1.56% 🔺)
import { useLoadableQuery } from "@apollo/client/react" (production)	3.43 KB (0%)
import { useReadQuery } from "@apollo/client/react"	6.78 KB (+1.93% 🔺)
import { useReadQuery } from "@apollo/client/react" (production)	1.66 KB (0%)
import { useFragment } from "@apollo/client/react"	6.84 KB (+1.97% 🔺)
import { useFragment } from "@apollo/client/react" (production)	1.71 KB (0%)

apollo-librarian · 2025-09-22T23:58:42Z

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: 7e3a261fa5d255f518c30676
Build Logs: View logs

jerelmiller · 2025-09-22T23:59:33Z

src/__tests__/local-state/general.ts

+  });
+
+  expect(merge).toHaveBeenCalledTimes(1);
+  expect(merge).toHaveBeenCalledWith(undefined, null, expect.anything());


If a merge function is present, we'll call it with null as the incoming value which I think makes sense since its resolved from LocalState. When a read function is the only thing defined, the merge function won't be called at all because the incoming value is set to undefined instead and merge functions aren't called in this case.

jerelmiller · 2025-09-23T00:45:06Z

@phryneas would love your initial input on this as a potential solution for the issue. I'll open this up and get ready for merge if you like the approach.

jerelmiller · 2025-09-23T21:46:38Z

.changeset/flat-worms-notice.md

@@ -0,0 +1,7 @@
+---
+"@apollo/client": minor


I marked this as minor (in which case we'll want to repoint the branch) since it feels like enough of a change to justify a minor. Happy to update to patch though if it feels more like a bug fix. Opinions welcome.

jerelmiller · 2025-09-23T21:46:57Z

.changeset/unlucky-cooks-rhyme.md

@@ -0,0 +1,7 @@
+---
+"@apollo/client": minor


Same. Would like opinions on whether this should be viewed as a minor or patch.

src/__tests__/local-state/general.ts

.changeset/unlucky-cooks-rhyme.md

phryneas · 2025-09-24T14:25:26Z

.changeset/shaggy-islands-yell.md

+"@apollo/client": patch
+---
+
+Warn when using a `no-cache` fetch policy when a `@client` field resolves the value from the cache that defines a `read` function.


Can you phrase this differently? Without context, this one might be very difficult to understand.

Yeesh this was horrible 😅. Updated in 0ff610d

src/local-state/LocalState.ts

phryneas · 2025-09-24T14:37:17Z

src/local-state/LocalState.ts

+                "The '%s' field resolves the value from the cache, for example from a 'read' function, but a 'no-cache' fetch policy was used. The field value has been set to `null`. Either define a local resolver or use a fetch policy that uses the cache to ensure the field is resolved correctly.",
+                resolverName
+              );
+              return null;


Do we want to return null here even in the returnPartialData: true case?

I initially thought yet, but I've changed my mind only because there is no other way to resolve the field value. no-cache and returnPartialData are technically incompatible together since no-cache queries don't read from the cache, so there isn't a possibility of a partial result. The same applies here since the value can only be resolved in LocalState when a no-cache query is issued since the read function isn't called. In this case, I think its better to have a null to ensure the query is complete since it can't be resolved any other way.

phryneas · 2025-09-24T14:38:05Z

src/local-state/LocalState.ts

+            }
+
+            // assume the cache will handle returning the correct value
+            returnPartialData = true;


Changing this here will disable the warning and null-returning behaviour a few lines down for all future fields. Do we want that to happen?
Might be necessary to create an immutable copy of the initial returnPartialData otherwise.

Discussed in person. This is caught by the conditional below that returns early when the value is null or undefined.

phryneas

Looks good now :)

jerelmiller added 3 commits September 22, 2025 17:24

Add failing tests for read/merge functions with local state

5c2627a

Add more test cases

c9974f7

Add partial solution

8e11de8

Update merge function test

281d673

jerelmiller commented Sep 22, 2025

View reviewed changes

jerelmiller added 3 commits September 22, 2025 18:00

Update comment

f418f0c

Pass fetchPolicy to localState for execute

19d993e

Add fetchPolicy to existing tests

6081aff

jerelmiller changed the title ~~[WIP] Alternate solution to local state value with read function~~ Alternate solution to local state value with read function Sep 23, 2025

jerelmiller requested a review from phryneas September 23, 2025 00:44

jerelmiller added 14 commits September 23, 2025 12:48

Add abstract resolvesClientField method to base cache

a2dace2

Implement resolvesClientField for inMemoryCache

4a9f40a

Add additional tests for warnings

9ec3941

Use cache.resolvesClientField in LocalState

2530b8b

Don't read from cache when using a no-cache fetch policy

826efef

Warn when using no-cache fetch policy with read function

6557f43

Remove else

896478b

Update local state tests with checks on warnings

fa5145a

Add changesets

dc29907

Update size limits

7b56986

Update API report

1950e6f

Tweak warnings

28f138d

Move warnings to testUtils

3fbf754

Update cache tests

4065958

jerelmiller changed the title ~~Alternate solution to local state value with read function~~ More robust handling of local state default value when read function is defined Sep 23, 2025

jerelmiller marked this pull request as ready for review September 23, 2025 21:45

jerelmiller commented Sep 23, 2025

View reviewed changes

jerelmiller added 2 commits September 23, 2025 15:50

Tweak tests

a976196

Update size limits

738659b

jerelmiller commented Sep 23, 2025

View reviewed changes

src/__tests__/local-state/general.ts Show resolved Hide resolved

jerelmiller mentioned this pull request Sep 23, 2025

Incorrect resolution of nested @client fields in v4 #12930

Closed

phryneas reviewed Sep 24, 2025

View reviewed changes

jerelmiller mentioned this pull request Sep 24, 2025

Add section in docs for child read functions with @client fields #12931

Closed

jerelmiller force-pushed the jerel/local-state-bug branch from 700836e to 01e239b Compare September 26, 2025 00:12

jerelmiller added 2 commits September 25, 2025 23:08

Tweak changeset

0888622

Remove unneeded destructure

846783f

jerelmiller requested a review from phryneas September 26, 2025 05:14

jerelmiller changed the base branch from main to release-4.1 September 26, 2025 05:14

jerelmiller force-pushed the jerel/local-state-bug branch from 57c43dd to 846783f Compare September 26, 2025 05:17

Formatting

4bceff8

phryneas approved these changes Sep 26, 2025

View reviewed changes

github-actions bot added the auto-cleanup 🤖 label Sep 26, 2025

jerelmiller merged commit 54ab6d9 into release-4.1 Sep 26, 2025
46 checks passed

jerelmiller deleted the jerel/local-state-bug branch September 26, 2025 14:57

github-actions bot mentioned this pull request Sep 26, 2025

Version Packages (alpha) #12940

Merged

github-actions bot locked as resolved and limited conversation to collaborators Oct 27, 2025

Conversation

jerelmiller commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

pkg-pr-new bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

apollo-librarian bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Docs preview has no changes

Uh oh!

jerelmiller Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerelmiller commented Sep 23, 2025

Uh oh!

jerelmiller Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

jerelmiller Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

phryneas Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

jerelmiller Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phryneas Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

jerelmiller Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

phryneas Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerelmiller Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

phryneas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jerelmiller commented Sep 22, 2025 •

edited

Loading

changeset-bot bot commented Sep 22, 2025 •

edited

Loading

pkg-pr-new bot commented Sep 22, 2025 •

edited

Loading

github-actions bot commented Sep 22, 2025 •

edited

Loading

apollo-librarian bot commented Sep 22, 2025 •

edited

Loading

jerelmiller Sep 22, 2025 •

edited

Loading

phryneas Sep 24, 2025 •

edited

Loading