Add overridable cache type - #13250
Conversation
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 3 changed, 0 removedBuild ID: 8f5e944383c23361471a8603 URL: https://www.apollographql.com/docs/deploy-preview/8f5e944383c23361471a8603 ✅ AI Style Review — No Changes DetectedNo MDX files were changed in this pull request. Review Log: View detailed log
|
🦋 Changeset detectedLatest commit: 8df9c40 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds a conditional type, Cache.Implementation (derived from TypeOverrides.cache or falling back to ApolloCache), updates ApolloClient core types and useMutation/react hook typings to use that type, updates deprecated aliases and API reports, and adds comprehensive integration type tests and TS project wiring. ChangesCache Type Override Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
|
|
||
| export type Implementation = | ||
| TypeOverrides extends { cache: infer TCache } ? | ||
| TCache extends ApolloCache ? |
There was a problem hiding this comment.
I chose to use ApolloCache as the type when the cache property is an invalid type (e.g. it isn't an ApolloCache subtype). Is this what we want? This "silently" fails in that the type is ignored completely and ApolloCache is used everywhere. Do we want this to be a bit more loud? If so, do we want to do like we did with default options and add a string message that points to our documentation?
Note: You can see the behavior in the cacheOverride/invalid/index.ts tests.
There was a problem hiding this comment.
No strong opinions but my two cents: the silent fallback could make someone waste time wondering why client.cache is still ApolloCache?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.api-reports/api-report-core.api.md (1)
219-263:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the deprecated top-level cache aliases as well.
The generated surface still exports
MutationOptions<TCache extends ApolloCache = ApolloCache>andRefetchQueriesOptions<TCache extends ApolloCache, TResult>later in this file (Lines 735 and 1227). That means consumers who still import those deprecated public names won't get the new override-aware cache typing even thoughApolloClient.MutateOptionsandApolloClient.RefetchQueriesOptionsnow do.Also applies to: 393-495
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.api-reports/api-report-core.api.md around lines 219 - 263, Update the deprecated top-level cache alias exports so they use the new override-aware cache implementation type instead of ApolloCache: replace the generic defaults on MutationOptions and RefetchQueriesOptions (and any other top-level aliases in the 393-495, 735, and 1227 ranges) so their TCache generic extends Cache_2.Implementation (or the same override-aware type used by ApolloClient.MutateOptions/ApolloClient.RefetchQueriesOptions) and mirror the new signatures/constraints used by the ApolloClient.* variants to ensure consumers importing the deprecated names get the same override-aware typing.
🧹 Nitpick comments (1)
integration-tests/type-tests/cacheOverride/invalid/index.ts (1)
31-49: ⚡ Quick winAdd a negative constructor check for the invalid override case.
This suite shows that exposed cache types fall back to
ApolloCache, but it never asserts thatTypeOverrides.cache = numberdoes not leak intoApolloClient.Options. A regression allowingcache: 123would still pass these tests.Proposed addition
test("ApolloClient constructor", () => { + { + const client = new ApolloClient({ + // `@ts-expect-error` cache must still be an ApolloCache implementation + cache: 123, + link: ApolloLink.empty(), + }); + + expectTypeOf(client.cache).toEqualTypeOf<ApolloCache>(); + } + { const client = new ApolloClient({ cache: new InMemoryCache(), link: ApolloLink.empty(), });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration-tests/type-tests/cacheOverride/invalid/index.ts` around lines 31 - 49, Add a negative TypeScript test to ensure TypeOverrides.cache does not leak into ApolloClient.Options by attempting to construct new ApolloClient with an invalid cache value and asserting a type error: in the "ApolloClient constructor" test add a case that passes cache: 123 (or another non-cache primitive) to ApolloClient constructor and mark it with a `@ts-expect-error` so the compiler fails the test if the override leaked; reference ApolloClient, ApolloClient.Options and TypeOverrides.cache when adding the check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integration-tests/type-tests/cacheOverride/classicSignature/index.ts`:
- Around line 1-8: The test file is instantiating new InMemoryCache() but
InMemoryCache is not imported; add InMemoryCache to the import list from
"`@apollo/client`" (the same import that currently brings in ApolloClient,
ApolloLink, Cache, TypedDocumentNode, MutationUpdaterFunction, ApolloCache) so
the symbol InMemoryCache is available for the instantiation in this file.
In `@integration-tests/type-tests/cacheOverride/modernSignature/index.ts`:
- Around line 1-8: The file uses InMemoryCache (seen in the expression cache:
new InMemoryCache()) but it isn't imported from `@apollo/client`; update the
import statement that currently lists ApolloClient, ApolloLink, Cache,
TypedDocumentNode, MutationUpdaterFunction, ApolloCache to also include
InMemoryCache so the type-test compiles and the usage of new InMemoryCache() is
resolved.
---
Outside diff comments:
In @.api-reports/api-report-core.api.md:
- Around line 219-263: Update the deprecated top-level cache alias exports so
they use the new override-aware cache implementation type instead of
ApolloCache: replace the generic defaults on MutationOptions and
RefetchQueriesOptions (and any other top-level aliases in the 393-495, 735, and
1227 ranges) so their TCache generic extends Cache_2.Implementation (or the same
override-aware type used by
ApolloClient.MutateOptions/ApolloClient.RefetchQueriesOptions) and mirror the
new signatures/constraints used by the ApolloClient.* variants to ensure
consumers importing the deprecated names get the same override-aware typing.
---
Nitpick comments:
In `@integration-tests/type-tests/cacheOverride/invalid/index.ts`:
- Around line 31-49: Add a negative TypeScript test to ensure
TypeOverrides.cache does not leak into ApolloClient.Options by attempting to
construct new ApolloClient with an invalid cache value and asserting a type
error: in the "ApolloClient constructor" test add a case that passes cache: 123
(or another non-cache primitive) to ApolloClient constructor and mark it with a
`@ts-expect-error` so the compiler fails the test if the override leaked;
reference ApolloClient, ApolloClient.Options and TypeOverrides.cache when adding
the check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: ec587630-aecf-4ff3-afb0-641f46ea3a52
📒 Files selected for processing (19)
.api-reports/api-report-cache.api.md.api-reports/api-report-core.api.md.api-reports/api-report-react.api.md.api-reports/api-report.api.md.changeset/fuzzy-hairs-tie.mdintegration-tests/type-tests/cacheOverride/classicSignature/index.tsintegration-tests/type-tests/cacheOverride/classicSignature/tsconfig.jsonintegration-tests/type-tests/cacheOverride/defaults/index.tsintegration-tests/type-tests/cacheOverride/defaults/tsconfig.jsonintegration-tests/type-tests/cacheOverride/invalid/index.tsintegration-tests/type-tests/cacheOverride/invalid/tsconfig.jsonintegration-tests/type-tests/cacheOverride/modernSignature/index.tsintegration-tests/type-tests/cacheOverride/modernSignature/tsconfig.jsonintegration-tests/type-tests/cacheOverride/shared/index.tsintegration-tests/type-tests/cacheOverride/shared/tsconfig.jsonintegration-tests/type-tests/tsconfig.jsonsrc/cache/core/types/Cache.tssrc/core/ApolloClient.tssrc/react/hooks/useMutation.ts
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This reverts commit fcaba7b.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to release-4.3, this PR will be updated.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ `release-4.3` is currently in **pre mode** so this branch has prereleases rather than normal releases. If you want to exit prereleases, run `changeset pre exit` on `release-4.3`.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ # Releases ## @apollo/client@4.3.0-alpha.0 ### Minor Changes - [#13250](#13250) [`bad7035`](bad7035) Thanks [@jerelmiller](https://github.com/jerelmiller)! - Add the ability to define the cache type for the client. `client.cache` currently returns `ApolloCache` as the cache type regardless of what cache you've provided to `ApolloClient`. Declare the cache type using the `cache` property in the `TypeOverrides` interface to set the cache implementation used for the client. ```ts // apollo.d.ts import type { InMemoryCache } from "@apollo/client"; declare module "@apollo/client" { export interface TypeOverrides { cache: InMemoryCache; } } ``` Now anywhere `cache` is accessible, the type is the declared cache type: ```ts client.cache; // ^? InMemoryCache client.mutate({ update: (cache) => { // ^? InMemoryCache }, }); ``` > [!NOTE] > Setting a cache type enforces that cache type in the `cache` option for the `ApolloClient` constructor. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
As a quick follow-up to PR #13250, this fixes some cache override typing paths that still allowed `ApolloCache`, while the public mutation and refetch APIs now expect `Cache.Implementation`. Keeping these constraints aligned ensures that user-declared cache overrides work consistently through `QueryInfo`, internal refetch options, and deprecated React mutation aliases. We also regenerated the API reports so that the published type surface shows the corrected constraints. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Aligned cache-related type constraints across the library for consistency. TypeScript users working with custom cache implementations may notice updated type signatures in mutation helpers and query-related APIs. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Currently
client.cachealways return anApolloCachetype, regardless of the actualcacheinstance you provide toApolloClient. This can be slightly annoying in cases where you might want to accesscachethroughclient.cacheand work directly with e.g.InMemoryCachespecific APIs. Currently this requires a type override, or access to the originalcacheinstance from elsewhere in your app.This PR adds the ability to specify the cache implementation in the
TypeOverridesinterface using thecacheproperty. Anywherecacheis accessible, its type will be the override type (e.g.client.cache // => InMemoryCache). To ensure the runtime value matches the declared type,ApolloClient.Optionshas been updated to enforce the implemented type.Summary by CodeRabbit
New Features
Tests
Documentation