Skip to content

Commit

Permalink
Determine queryInfo.queryId in QueryInfo constructor.
Browse files Browse the repository at this point in the history
This refactoring ensures `observableQuery.queryId` ends up the same as
`observableQuery.queryInfo.queryId`, which is relevant in cases when we
choose a specific query ID string before creating the `QueryInfo` and
`ObservableQuery` objects.

Ensuring that `queryInfo.queryId === queryInfo.observableQuery.queryId`
(another way of saying the same thing) also fixes the `invariant`
failures I deliberately introduced in the previous commit.

This inconsistency between IDs was the cause of issue #8586, a double
registration bug for mutation `refetchQueries` specified using the
legacy one-time `refetchQueries: [{ query, variables }]` style.

Another (recommended) way to work around that problem would be to
specify `refetchQueries: [query]` instead, to select the existing query
by its `DocumentNode` rather than creating and deleting a new one-time
legacy query.
  • Loading branch information
benjamn committed Aug 4, 2021
1 parent b1b0b37 commit ae8355a
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export class ObservableQuery<

// query information
this.options = options;
this.queryId = queryManager.generateQueryId();
this.queryId = queryInfo.queryId || queryManager.generateQueryId();

const opDef = getOperationDefinition(options.query);
this.queryName = opDef && opDef.name && opDef.name.value;
Expand Down
10 changes: 9 additions & 1 deletion src/core/QueryInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
isNetworkRequestInFlight,
} from './networkStatus';
import { ApolloError } from '../errors';
import { QueryManager } from './QueryManager';

export type QueryStoreValue = Pick<QueryInfo,
| "variables"
Expand Down Expand Up @@ -85,7 +86,14 @@ export class QueryInfo {
graphQLErrors?: ReadonlyArray<GraphQLError>;
stopped = false;

constructor(private cache: ApolloCache<any>) {
private cache: ApolloCache<any>;

constructor(
queryManager: QueryManager<any>,
public readonly queryId = queryManager.generateQueryId(),
) {
const cache = this.cache = queryManager.cache;

// Track how often cache.evict is called, since we want eviction to
// override the feud-stopping logic in the markResult method, by
// causing shouldWrite to return true. Wrapping the cache.evict method
Expand Down
4 changes: 2 additions & 2 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ export class QueryManager<TStore> {
options.notifyOnNetworkStatusChange = false;
}

const queryInfo = new QueryInfo(this.cache);
const queryInfo = new QueryInfo(this);
const observable = new ObservableQuery<T, TVariables>({
queryManager: this,
queryInfo,
Expand Down Expand Up @@ -1470,7 +1470,7 @@ export class QueryManager<TStore> {

private getQuery(queryId: string): QueryInfo {
if (queryId && !this.queries.has(queryId)) {
this.queries.set(queryId, new QueryInfo(this.cache));
this.queries.set(queryId, new QueryInfo(this, queryId));
}
return this.queries.get(queryId)!;
}
Expand Down

0 comments on commit ae8355a

Please sign in to comment.