Skip to content
Merged
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
31 changes: 31 additions & 0 deletions src/cache/inmemory/__tests__/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,37 @@ describe("reading from the store", () => {
});
});

it("applies defaults when variable is explicitly undefined", () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great test! Can we also verify it correctly uses defaults if show is omitted altogether as well? I don't think we have any other test in this suite that verifies that. That just makes sure we don't add any regressions there as well 🙂

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the test doesn't work at all since the field has no value in the store. So even if show: true was used (or defaulted), the query would return only { id: "abcd" } as the result?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ab-pm the readQueryFromStore helper that this test uses sets returnPartialData: false by default so if @include really weren't evaluated, the result would be null instead of { id: "abcd" } due to the result being a partial result. I think its ok 🙂

const query = gql`
query someQuery($show: Boolean = false) {
id
field @include(if: $show)
}
`;

const variables = {
show: undefined,
};

const store = defaultNormalizedCacheFactory({
ROOT_QUERY: {
__typename: "Query",
id: "abcd",
} as StoreObject,
});

const result = readQueryFromStore(reader, {
store,
query,
variables,
});

// $show defaults to false, so @include(if: false) excludes the field
expect(result).toEqual({
id: "abcd",
});
});

it("runs a nested query", () => {
const result: any = {
id: "abcd",
Expand Down
5 changes: 4 additions & 1 deletion src/cache/inmemory/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,12 @@ export class StoreReader {
}: DiffQueryAgainstStoreOptions): Cache.DiffResult<T> {
const policies = this.config.cache.policies;

const rawVariables = variables ?? {};
variables = {
...getDefaultValues(getQueryDefinition(query)),
...variables!,
...Object.fromEntries(
Object.entries(rawVariables).filter(([, v]) => v !== undefined)
),
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rather than spreading variables, then spreading it again (with a filter applied), lets instead use the compact utility which merges objects except undefined values.

Suggested change
const rawVariables = variables ?? {};
variables = {
...getDefaultValues(getQueryDefinition(query)),
...variables!,
...Object.fromEntries(
Object.entries(rawVariables).filter(([, v]) => v !== undefined)
),
};
variables = compact(
getDefaultValues(getQueryDefinition(query)),
variables
);


const rootRef = makeReference(rootId);
Expand Down