Skip to content

Commit

Permalink
fix: call iterateObserversSafely if variables change
Browse files Browse the repository at this point in the history
  • Loading branch information
alessbell committed Sep 29, 2022
1 parent 184ebc0 commit 8412af2
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 3 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
{
"name": "apollo-client",
"path": "./dist/apollo-client.min.cjs",
"maxSize": "31.71kB"
"maxSize": "31.74kB"
}
],
"engines": {
Expand Down
109 changes: 108 additions & 1 deletion src/__tests__/local-state/general.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
import gql from 'graphql-tag';
import { DocumentNode, GraphQLError, getIntrospectionQuery } from 'graphql';
import {
graphql,
GraphQLInt,
print,
DocumentNode,
GraphQLError,
getIntrospectionQuery,
GraphQLSchema,
GraphQLObjectType,
GraphQLID,
GraphQLString
} from 'graphql';

import { Observable } from '../../utilities';
import { ApolloLink } from '../../link/core';
Expand Down Expand Up @@ -819,6 +830,102 @@ describe('Combining client and server state/operations', () => {
}, 10);
});

itAsync('query resolves with loading: false if subsequent responses contain the same data', (resolve, reject) => {
const request = {
query: gql`
query people($id: Int) {
people(id: $id) {
id
name
}
}
`,
variables: {
id: 1,
},
notifyOnNetworkStatusChange: true
};

const PersonType = new GraphQLObjectType({
name: "Person",
fields: {
id: { type: GraphQLID },
name: { type: GraphQLString }
}
});

const peopleData = [
{ id: 1, name: "John Smith" },
{ id: 2, name: "Sara Smith" },
{ id: 3, name: "Budd Deey" }
];

const QueryType = new GraphQLObjectType({
name: "Query",
fields: {
people: {
type: PersonType,
args: {
id: {
type: GraphQLInt
}
},
resolve: (_, { id }) => {
return peopleData;
}
}
}
});

const schema = new GraphQLSchema({ query: QueryType });

const link = new ApolloLink(operation => {
// @ts-ignore
return new Observable(async observer => {
const { query, operationName, variables } = operation;
try {
const result = await graphql({
schema,
source: print(query),
variableValues: variables,
operationName,
});
observer.next(result);
observer.complete();
} catch (err) {
observer.error(err);
}
});
});

const client = new ApolloClient({
cache: new InMemoryCache(),
link,
});

const observer = client.watchQuery(request);

let count = 0;
observer.subscribe({
next: ({ loading, data }) => {
if (count === 0) expect(loading).toBe(false);
if (count === 1) expect(loading).toBe(true);
if (count === 2) {
expect(loading).toBe(false)
resolve();
};
count++;
},
error: reject,
});

setTimeout(() => {
observer.refetch({
id: 2
});
}, 1);
});

itAsync('should correctly propagate an error from a client resolver', async (resolve, reject) => {
const data = {
list: {
Expand Down
10 changes: 9 additions & 1 deletion src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,12 @@ export class ObservableQuery<
return !this.last || !equal(this.last.result, newResult);
}

// Compares variables to the variables in the snapshot we took of
// this.lastResult when it was first received.
public hasDifferentVariablesFromLastResult(variables?: TVariables) {
return !this.last || !equal(this.last.variables, variables)
}

private getLast<K extends keyof Last<TData, TVariables>>(
key: K,
variablesMustMatch?: boolean,
Expand Down Expand Up @@ -872,7 +878,9 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
variables: TVariables | undefined,
) {
const lastError = this.getLastError();
if (lastError || this.isDifferentFromLastResult(result)) {
if (lastError
|| this.isDifferentFromLastResult(result)
|| this.hasDifferentVariablesFromLastResult(variables)) {
if (lastError || !result.partial || this.options.returnPartialData) {
this.updateLastResult(result, variables);
}
Expand Down
10 changes: 10 additions & 0 deletions src/core/__tests__/fetchPolicies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,17 @@ describe("nextFetchPolicy", () => {
// resets the fetchPolicy to context.initialPolicy), so cache-first is
// still what we see here.
expect(observable.options.fetchPolicy).toBe("cache-first");
} else if (count === 3) {
expect(result.loading).toBe(false);
expect(result.data).toEqual({
linkCounter: 2,
opName: "EchoQuery",
opVars: {
refetching: true,
},
});

expect(observable.options.fetchPolicy).toBe("cache-first");
setTimeout(resolve, 20);
} else {
reject(`Too many results (${count})`);
Expand Down

0 comments on commit 8412af2

Please sign in to comment.