diff --git a/CHANGELOG.md b/CHANGELOG.md
index d1148691e61..37faf75c9ba 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,10 @@
+## Apollo Client 3.4.10 (not yet released)
+
+### Improvements
+
+- Warn when calling `refetch({ variables })` instead of `refetch(variables)`, except for queries that declare a variable named `$variables` (uncommon).
+ [@benjamn](https://github.com/benjamn) in [#8702](https://github.com/apollographql/apollo-client/pull/8702)
+
## Apollo Client 3.4.9
### Bug Fixes
diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts
index 6038a87755f..9bd4808a5fb 100644
--- a/src/core/ObservableQuery.ts
+++ b/src/core/ObservableQuery.ts
@@ -13,6 +13,7 @@ import {
iterateObserversSafely,
isNonEmptyArray,
fixObservableSubclass,
+ getQueryDefinition,
} from '../utilities';
import { ApolloError } from '../errors';
import { QueryManager } from './QueryManager';
@@ -26,6 +27,11 @@ import {
import { QueryInfo } from './QueryInfo';
import { MissingFieldError } from '../cache';
+const {
+ assign,
+ hasOwnProperty,
+} = Object;
+
export interface FetchMoreOptions<
TData = any,
TVariables = OperationVariables
@@ -311,6 +317,19 @@ export class ObservableQuery<
reobserveOptions.fetchPolicy = 'network-only';
}
+ if (__DEV__ && variables && hasOwnProperty.call(variables, "variables")) {
+ const queryDef = getQueryDefinition(this.options.query);
+ const vars = queryDef.variableDefinitions;
+ if (!vars || !vars.some(v => v.variable.name.value === "variables")) {
+ invariant.warn(`Called refetch(${
+ JSON.stringify(variables)
+ }) for query ${
+ queryDef.name?.value || JSON.stringify(queryDef)
+ }, which does not declare a $variables variable.
+Did you mean to call refetch(variables) instead of refetch({ variables })?`);
+ }
+ }
+
if (variables && !equal(this.options.variables, variables)) {
// Update the existing options with new variables
reobserveOptions.variables = this.options.variables = {
@@ -657,7 +676,7 @@ once, rather than every time you call fetchMore.`);
// Disposable Concast fetches receive a shallow copy of this.options
// (merged with newOptions), leaving this.options unmodified.
? compact(this.options, newOptions)
- : Object.assign(this.options, compact(newOptions));
+ : assign(this.options, compact(newOptions));
if (!useDisposableConcast) {
// We can skip calling updatePolling if we're not changing this.options.
diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts
index 94bea64470a..4111121355b 100644
--- a/src/core/__tests__/ObservableQuery.ts
+++ b/src/core/__tests__/ObservableQuery.ts
@@ -1365,6 +1365,252 @@ describe('ObservableQuery', () => {
},
});
});
+
+ describe("warnings about refetch({ variables })", () => {
+ itAsync("should warn if passed { variables } and query does not declare any variables", (resolve, reject) => {
+ const consoleWarnSpy = jest.spyOn(console, "warn");
+ consoleWarnSpy.mockImplementation(() => {});
+
+ const queryWithoutVariables = gql`
+ query QueryWithoutVariables {
+ getVars {
+ __typename
+ name
+ }
+ }
+ `;
+
+ function makeMock(...vars: string[]) {
+ const requestWithoutVariables = {
+ query: queryWithoutVariables,
+ variables: {
+ variables: vars,
+ },
+ };
+
+ const resultWithVariables = {
+ data: {
+ getVars: vars.map(name => ({
+ __typename: "Var",
+ name,
+ })),
+ },
+ };
+
+ return {
+ request: requestWithoutVariables,
+ result: resultWithVariables,
+ };
+ }
+
+ const observableWithoutVariables: ObservableQuery = mockWatchQuery(
+ reject,
+ makeMock("a", "b", "c"),
+ makeMock("d", "e"),
+ );
+
+ subscribeAndCount(reject, observableWithoutVariables, (count, result) => {
+ expect(result.error).toBeUndefined();
+
+ if (count === 1) {
+ expect(result.loading).toBe(false);
+ expect(result.data).toEqual({
+ getVars: [
+ { __typename: "Var", name: "a" },
+ { __typename: "Var", name: "b" },
+ { __typename: "Var", name: "c" },
+ ],
+ });
+
+ // It's a common mistake to call refetch({ variables }) when you meant
+ // to call refetch(variables).
+ observableWithoutVariables.refetch({
+ variables: ["d", "e"],
+ }).catch(reject);
+
+ } else if (count === 2) {
+ expect(result.loading).toBe(false);
+ expect(result.data).toEqual({
+ getVars: [
+ { __typename: "Var", name: "d" },
+ { __typename: "Var", name: "e" },
+ ],
+ });
+
+ expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
+ expect(consoleWarnSpy).toHaveBeenCalledWith([
+ 'Called refetch({"variables":["d","e"]}) for query QueryWithoutVariables, which does not declare a $variables variable.',
+ "Did you mean to call refetch(variables) instead of refetch({ variables })?",
+ ].join("\n"));
+ consoleWarnSpy.mockReset();
+
+ setTimeout(resolve, 10);
+ } else {
+ reject(`too many results (${count})`);
+ }
+ });
+ });
+
+ itAsync("should warn if passed { variables } and query does not declare $variables", (resolve, reject) => {
+ const consoleWarnSpy = jest.spyOn(console, "warn");
+ consoleWarnSpy.mockImplementation(() => {});
+
+ const queryWithVarsVar = gql`
+ query QueryWithVarsVar($vars: [String!]) {
+ getVars(variables: $vars) {
+ __typename
+ name
+ }
+ }
+ `;
+
+ function makeMock(...vars: string[]) {
+ const requestWithVarsVar = {
+ query: queryWithVarsVar,
+ variables: { vars },
+ };
+
+ const resultWithVarsVar = {
+ data: {
+ getVars: vars.map(name => ({
+ __typename: "Var",
+ name,
+ })),
+ },
+ };
+
+ return {
+ request: requestWithVarsVar,
+ result: resultWithVarsVar,
+ };
+ }
+
+ const observableWithVarsVar: ObservableQuery = mockWatchQuery(
+ reject,
+ makeMock("a", "b", "c"),
+ makeMock("d", "e"),
+ );
+
+ subscribeAndCount(error => {
+ expect(error.message).toMatch(
+ 'No more mocked responses for the query: query QueryWithVarsVar($vars: [String!])'
+ );
+ }, observableWithVarsVar, (count, result) => {
+ expect(result.error).toBeUndefined();
+
+ if (count === 1) {
+ expect(result.loading).toBe(false);
+ expect(result.data).toEqual({
+ getVars: [
+ { __typename: "Var", name: "a" },
+ { __typename: "Var", name: "b" },
+ { __typename: "Var", name: "c" },
+ ],
+ });
+
+ // It's a common mistake to call refetch({ variables }) when you meant
+ // to call refetch(variables).
+ observableWithVarsVar.refetch({
+ variables: { vars: ["d", "e"] },
+ }).then(result => {
+ reject(`unexpected result ${JSON.stringify(result)}; should have thrown`);
+ }, error => {
+ expect(error.message).toMatch(
+ 'No more mocked responses for the query: query QueryWithVarsVar($vars: [String!])'
+ );
+ expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
+ expect(consoleWarnSpy).toHaveBeenCalledWith([
+ 'Called refetch({"variables":{"vars":["d","e"]}}) for query QueryWithVarsVar, which does not declare a $variables variable.',
+ "Did you mean to call refetch(variables) instead of refetch({ variables })?",
+ ].join("\n"));
+ consoleWarnSpy.mockReset();
+
+ setTimeout(resolve, 10);
+ });
+
+ } else {
+ reject(`one too many (${count}) results: ${JSON.stringify(result)}`);
+ }
+ });
+ });
+
+ itAsync("should not warn if passed { variables } and query declares $variables", (resolve, reject) => {
+ const consoleWarnSpy = jest.spyOn(console, "warn");
+ consoleWarnSpy.mockImplementation(() => {});
+
+ const queryWithVariablesVar = gql`
+ query QueryWithVariablesVar($variables: [String!]) {
+ getVars(variables: $variables) {
+ __typename
+ name
+ }
+ }
+ `;
+
+ function makeMock(...variables: string[]) {
+ const requestWithVariablesVar = {
+ query: queryWithVariablesVar,
+ variables: {
+ variables,
+ },
+ };
+
+ const resultWithVariablesVar = {
+ data: {
+ getVars: variables.map(name => ({
+ __typename: "Var",
+ name,
+ })),
+ },
+ };
+
+ return {
+ request: requestWithVariablesVar,
+ result: resultWithVariablesVar,
+ };
+ }
+
+ const observableWithVariablesVar: ObservableQuery = mockWatchQuery(
+ reject,
+ makeMock("a", "b", "c"),
+ makeMock("d", "e"),
+ );
+
+ subscribeAndCount(reject, observableWithVariablesVar, (count, result) => {
+ expect(result.error).toBeUndefined();
+ if (count === 1) {
+ expect(result.loading).toBe(false);
+ expect(result.data).toEqual({
+ getVars: [
+ { __typename: "Var", name: "a" },
+ { __typename: "Var", name: "b" },
+ { __typename: "Var", name: "c" },
+ ],
+ });
+
+ observableWithVariablesVar.refetch({
+ variables: ["d", "e"],
+ }).catch(reject);
+
+ } else if (count === 2) {
+ expect(result.loading).toBe(false);
+ expect(result.data).toEqual({
+ getVars: [
+ { __typename: "Var", name: "d" },
+ { __typename: "Var", name: "e" },
+ ],
+ });
+
+ expect(consoleWarnSpy).not.toHaveBeenCalled();
+ consoleWarnSpy.mockReset();
+
+ setTimeout(resolve, 10);
+ } else {
+ reject(`too many results (${count})`);
+ }
+ });
+ });
+ });
});
describe('currentResult', () => {