Skip to content

InMemoryCache: store fields with an empty object of optional arguments the same as fields without arguments #12370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changeset/late-bottles-add.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
"@apollo/client": patch
---

`InMemoryCache`: Fields with an empty argument object are now saved the same way as fields without arguments.

Previously, it was possible that the reponses for these two queries would be stored differently in the cache:

```gql
query PlainAccess {
myField
}
```
would be stored as `myField`
and
```gql
query AccessWithoutOptionalArgument($optional: String) {
myField(optional: $optional)
}
```
would be stored as `myField({"optional":"Foo"})` if called with `{optional: "Foo"}` and as `myField({})` if called without the optional argument.

The cases `myField` and `myField({})` are equivalent from the perspective of a GraphQL server, and so in the future both of these will be stored as `myField` in the cache.
12 changes: 6 additions & 6 deletions src/cache/inmemory/__tests__/entityStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1422,7 +1422,7 @@ describe("EntityStore", () => {
name: "Isaac Asimov",
hobby: "chemistry",
},
"authorOfBook({})": {
authorOfBook: {
__typename: "Author",
name: "James S.A. Corey",
hobby: "tabletop games",
Expand All @@ -1443,7 +1443,7 @@ describe("EntityStore", () => {
name: "Isaac Asimov",
hobby: "chemistry",
},
"authorOfBook({})": {
authorOfBook: {
__typename: "Author",
name: "James S.A. Corey",
hobby: "tabletop games",
Expand All @@ -1464,7 +1464,7 @@ describe("EntityStore", () => {
name: "Isaac Asimov",
hobby: "chemistry",
},
"authorOfBook({})": {
authorOfBook: {
__typename: "Author",
name: "James S.A. Corey",
hobby: "tabletop games",
Expand Down Expand Up @@ -1570,7 +1570,7 @@ describe("EntityStore", () => {
name: "Isaac Asimov",
hobby: "chemistry",
},
"authorOfBook({})": {
authorOfBook: {
__typename: "Author",
name: "James S.A. Corey",
hobby: "tabletop games",
Expand All @@ -1592,7 +1592,7 @@ describe("EntityStore", () => {
name: "Isaac Asimov",
hobby: "chemistry",
},
"authorOfBook({})": {
authorOfBook: {
__typename: "Author",
name: "James S.A. Corey",
hobby: "tabletop games",
Expand All @@ -1614,7 +1614,7 @@ describe("EntityStore", () => {
name: "Isaac Asimov",
hobby: "chemistry",
},
"authorOfBook({})": {
authorOfBook: {
__typename: "Author",
name: "James S.A. Corey",
hobby: "tabletop games",
Expand Down
8 changes: 5 additions & 3 deletions src/cache/inmemory/entityStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ export abstract class EntityStore implements NormalizedCache {

public modify(
dataId: string,
fields: Modifier<any> | Modifiers<Record<string, any>>
fields: Modifier<any> | Modifiers<Record<string, any>>,
exact: boolean
Copy link
Member Author

@phryneas phryneas Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opted to add exact only to our internal api and not expose it.
Userland code can still compare the storeFieldName and fieldName values passed into the modifier function to reach this goal, and we should probably not encourage people to write code like

cache.modify(id, {
  fields: {
    ['myField({"complex":....})']() {
      /* ... */
    },
  },
});

in the first place.
There is would be slightly more sane to use

cache.modify(id, {
  fields: {
    myField(value, { storeFieldName }) {
      if (storeFieldName === 'myField({"complex":....})') {
        /* ... */
      }
    },
  },
});

instead.

): boolean {
const storeObject = this.lookup(dataId);

Expand Down Expand Up @@ -238,7 +239,7 @@ export abstract class EntityStore implements NormalizedCache {
if (fieldValue === void 0) return;
const modify: Modifier<StoreValue> | undefined =
typeof fields === "function" ? fields : (
fields[storeFieldName] || fields[fieldName]
fields[storeFieldName] || (exact ? undefined : fields[fieldName])
);
if (modify) {
let newValue =
Expand Down Expand Up @@ -354,7 +355,8 @@ export abstract class EntityStore implements NormalizedCache {
{
[storeFieldName]: delModifier,
}
: delModifier
: delModifier,
args ? true : false
);
}
return false;
Expand Down
2 changes: 1 addition & 1 deletion src/cache/inmemory/inMemoryCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
: this.data;
try {
++this.txCount;
return store.modify(options.id || "ROOT_QUERY", options.fields);
return store.modify(options.id || "ROOT_QUERY", options.fields, false);
} finally {
if (!--this.txCount && options.broadcast !== false) {
this.broadcastWatches();
Expand Down
3 changes: 2 additions & 1 deletion src/cache/inmemory/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ export interface NormalizedCache {

modify<Entity extends Record<string, any>>(
dataId: string,
fields: Modifiers<Entity> | AllFieldsModifier<Entity>
fields: Modifiers<Entity> | AllFieldsModifier<Entity>,
exact: boolean
): boolean;
delete(dataId: string, fieldName?: string): boolean;
clear(): void;
Expand Down
14 changes: 8 additions & 6 deletions src/utilities/graphql/storeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,12 +245,12 @@ export const getStoreKeyName = Object.assign(
filteredArgs[key] = args[key];
});

return `${directives["connection"]["key"]}(${storeKeyNameStringify(
filteredArgs
)})`;
} else {
return directives["connection"]["key"];
const stringifiedArgs: string = storeKeyNameStringify(filteredArgs);
if (stringifiedArgs !== "{}") {
return `${directives["connection"]["key"]}(${stringifiedArgs})`;
}
}
return directives["connection"]["key"];
}

let completeFieldName: string = fieldName;
Expand All @@ -260,7 +260,9 @@ export const getStoreKeyName = Object.assign(
// and can lead to different store key names being created even though
// the `args` object used during creation has the same properties/values.
const stringifiedArgs: string = storeKeyNameStringify(args);
completeFieldName += `(${stringifiedArgs})`;
if (stringifiedArgs !== "{}") {
completeFieldName += `(${stringifiedArgs})`;
}
}

if (directives) {
Expand Down
Loading