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 Oct 12, 2022
1 parent c915214 commit 94256e6
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 4 deletions.
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
13 changes: 10 additions & 3 deletions src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,15 @@ export class ObservableQuery<

// Compares newResult to the snapshot we took of this.lastResult when it was
// first received.
public isDifferentFromLastResult(newResult: ApolloQueryResult<TData>) {
return !this.last || !equal(this.last.result, newResult);
public isDifferentFromLastResult(
newResult: ApolloQueryResult<TData>,
variables?: TVariables
) {
return (
!this.last ||
!equal(this.last.result, newResult) ||
!equal(this.last.variables, variables)
);
}

private getLast<K extends keyof Last<TData, TVariables>>(
Expand Down Expand Up @@ -872,7 +879,7 @@ 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, 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 94256e6

Please sign in to comment.