Skip to content

Commit

Permalink
Return { concast, fromLink } objects instead of flat tuples.
Browse files Browse the repository at this point in the history 
#10597 (comment)
  • Loading branch information
@benjamn
benjamn committed Mar 27, 2023
1 parent 83db291 commit 18cb2f3
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 38 deletions.
8 changes: 4 additions & 4 deletions src/core/ObservableQuery.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -692,11 +692,11 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
private fetch(
options: WatchQueryOptions<TVariables, TData>,
newNetworkStatus?: NetworkStatus,
): [concast: Concast<ApolloQueryResult<TData>>, containsDataFromLink: boolean] {
) {
// TODO Make sure we update the networkStatus (and infer fetchVariables)
// before actually committing to the fetch.
this.queryManager.setObservableQuery(this);
return this.queryManager['fetchQueryObservableWithInfo'](
return this.queryManager['fetchConcastWithInfo'](
this.queryId,
options,
newNetworkStatus,
Expand Down Expand Up @@ -835,7 +835,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
}

const variables = options.variables && { ...options.variables };
const [concast, containsDataFromLink] = this.fetch(options, newNetworkStatus);
const { concast, fromLink } = this.fetch(options, newNetworkStatus);
const observer: Observer<ApolloQueryResult<TData>> = {
next: result => {
this.reportResult(result, variables);
Expand All @@ -845,7 +845,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
},
};

if (!useDisposableConcast && containsDataFromLink) {
if (!useDisposableConcast && fromLink) {
// We use the {add,remove}Observer methods directly to avoid wrapping
// observer with an unnecessary SubscriptionObserver object.
if (this.concast && this.observer) {
Expand Down
78 changes: 49 additions & 29 deletions src/core/QueryManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1159,20 +1159,23 @@ export class QueryManager<TStore> {
// The initial networkStatus for this fetch, most often
// NetworkStatus.loading, but also possibly fetchMore, poll, refetch,
// or setVariables.
networkStatus = NetworkStatus.loading
networkStatus?: NetworkStatus,
): Concast<ApolloQueryResult<TData>> {
const [concast] = this.fetchQueryObservableWithInfo(queryId, options, networkStatus);
return concast;
return this.fetchConcastWithInfo(
queryId,
options,
networkStatus,
).concast;
}

private fetchQueryObservableWithInfo<TData, TVars extends OperationVariables>(
private fetchConcastWithInfo<TData, TVars extends OperationVariables>(
queryId: string,
options: WatchQueryOptions<TVars, TData>,
// The initial networkStatus for this fetch, most often
// NetworkStatus.loading, but also possibly fetchMore, poll, refetch,
// or setVariables.
networkStatus = NetworkStatus.loading
): [concast: Concast<ApolloQueryResult<TData>>, containsDataFromLink: boolean] {
): ConcastAndInfo<TData> {
const query = this.transform(options.query).document;
const variables = this.getVariables(query, options.variables) as TVars;
const queryInfo = this.getQuery(queryId);
Expand Down Expand Up @@ -1202,7 +1205,7 @@ export class QueryManager<TStore> {
// WatchQueryOptions object.
normalized.variables = variables;

const infoAndSources = this.fetchQueryByPolicy<TData, TVars>(
const sourcesWithInfo = this.fetchQueryByPolicy<TData, TVars>(
queryInfo,
normalized,
networkStatus,
Expand All @@ -1214,13 +1217,13 @@ export class QueryManager<TStore> {
normalized.fetchPolicy !== "standby" &&
// The "standby" policy currently returns [] from fetchQueryByPolicy, so
// this is another way to detect when nothing was done/fetched.
infoAndSources.length > 1 &&
sourcesWithInfo.sources.length > 0 &&
queryInfo.observableQuery
) {
queryInfo.observableQuery["applyNextFetchPolicy"]("after-fetch", options);
}

return infoAndSources;
return sourcesWithInfo;
};

// This cancel function needs to be set before the concast is created,
Expand All @@ -1244,7 +1247,7 @@ export class QueryManager<TStore> {
concast = new Concast(
this.localState
.addExportedVariables(normalized.query, normalized.variables, normalized.context)
.then(fromVariables).then(([_, ...sources]) => sources)
.then(fromVariables).then(sourcesWithInfo => sourcesWithInfo.sources),
);
// there is just no way we can synchronously get the *right* value here,
// so we will assume `true`, which is the behaviour before the bug fix in
Expand All @@ -1253,14 +1256,17 @@ export class QueryManager<TStore> {
// directives.
containsDataFromLink = true;
} else {
let sources: ConcastSourcesArray<ApolloQueryResult<TData>>;
[containsDataFromLink, ...sources] = fromVariables(normalized.variables);
concast = new Concast(sources);
const sourcesWithInfo = fromVariables(normalized.variables);
containsDataFromLink = sourcesWithInfo.fromLink;
concast = new Concast(sourcesWithInfo.sources);
}

concast.promise.then(cleanupCancelFn, cleanupCancelFn);

return [concast, containsDataFromLink];
return {
concast,
fromLink: containsDataFromLink,
};
}

public refetchQueries<TResult>({
Expand Down Expand Up @@ -1434,7 +1440,7 @@ export class QueryManager<TStore> {
// NetworkStatus.loading, but also possibly fetchMore, poll, refetch,
// or setVariables.
networkStatus: NetworkStatus
): [containsDataFromLink: boolean, ...sources: ConcastSourcesArray<ApolloQueryResult<TData>>] {
): SourcesAndInfo<TData> {
const oldNetworkStatus = queryInfo.networkStatus;

queryInfo.init({
Expand Down Expand Up @@ -1520,52 +1526,54 @@ export class QueryManager<TStore> {
const diff = readCache();

if (diff.complete) {
return [false, resultsFromCache(diff, queryInfo.markReady())];
return { fromLink: false, sources: [resultsFromCache(diff, queryInfo.markReady())] };
}

if (returnPartialData || shouldNotify) {
return [true, resultsFromCache(diff), resultsFromLink()];
return { fromLink: true, sources: [resultsFromCache(diff), resultsFromLink()] };
}

return [true, resultsFromLink()];
return { fromLink: true, sources: [resultsFromLink()] };
}

case "cache-and-network": {
const diff = readCache();

if (diff.complete || returnPartialData || shouldNotify) {
return [true, resultsFromCache(diff), resultsFromLink()];
return { fromLink: true, sources: [resultsFromCache(diff), resultsFromLink()] };
}

return [true, resultsFromLink()];
return { fromLink: true, sources: [resultsFromLink()] };
}

case "cache-only":
return [false, resultsFromCache(readCache(), queryInfo.markReady())];
return { fromLink: false, sources: [resultsFromCache(readCache(), queryInfo.markReady())] };

case "network-only":
if (shouldNotify) {
return [true, resultsFromCache(readCache()), resultsFromLink()];
return { fromLink: true, sources: [resultsFromCache(readCache()), resultsFromLink()] };
}

return [true, resultsFromLink()];
return { fromLink: true, sources: [resultsFromLink()] };

case "no-cache":
if (shouldNotify) {
return [
true,
return {
fromLink: true,
// Note that queryInfo.getDiff() for no-cache queries does not call
// cache.diff, but instead returns a { complete: false } stub result
// when there is no queryInfo.diff already defined.
resultsFromCache(queryInfo.getDiff()),
resultsFromLink(),
];
sources: [
resultsFromCache(queryInfo.getDiff()),
resultsFromLink(),
],
};
}

return [true, resultsFromLink()];
return { fromLink: true, sources: [resultsFromLink()] };

case "standby":
return [false];
return { fromLink: false, sources: [] };
}
}

Expand All @@ -1584,3 +1592,15 @@ export class QueryManager<TStore> {
};
}
}

// Return types used by fetchQueryByPolicy and other private methods above.
interface FetchConcastInfo {
// Metadata properties that can be returned in addition to the Concast.
fromLink: boolean;
}
interface SourcesAndInfo<TData> extends FetchConcastInfo {
sources: ConcastSourcesArray<ApolloQueryResult<TData>>;
}
interface ConcastAndInfo<TData> extends FetchConcastInfo {
concast: Concast<ApolloQueryResult<TData>>;
}
10 changes: 5 additions & 5 deletions src/core/__tests__/ObservableQuery.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@ import { waitFor } from "@testing-library/react";

export const mockFetchQuery = (queryManager: QueryManager<any>) => {
const fetchQueryObservable = queryManager.fetchQueryObservable;
const fetchQueryObservableWithInfo = queryManager['fetchQueryObservableWithInfo'];
const fetchConcastWithInfo = queryManager['fetchConcastWithInfo'];
const fetchQueryByPolicy: QueryManager<any>["fetchQueryByPolicy"] = (queryManager as any)
.fetchQueryByPolicy;

const mock = <T extends typeof fetchQueryObservable | typeof fetchQueryObservableWithInfo | typeof fetchQueryByPolicy>(original: T) =>
const mock = <T extends typeof fetchQueryObservable | typeof fetchConcastWithInfo | typeof fetchQueryByPolicy>(original: T) =>
jest.fn<ReturnType<T>, Parameters<T>>(function () {
return original.apply(queryManager, arguments);
});

const mocks = {
fetchQueryObservable: mock(fetchQueryObservable),
fetchQueryObservableWithInfo: mock(fetchQueryObservableWithInfo),
fetchConcastWithInfo: mock(fetchConcastWithInfo),
fetchQueryByPolicy: mock(fetchQueryByPolicy),
};

Expand Down Expand Up @@ -1002,7 +1002,7 @@ describe("ObservableQuery", () => {
expect(fqbpCalls[0][1].fetchPolicy).toEqual("cache-first");
expect(fqbpCalls[1][1].fetchPolicy).toEqual("network-only");

const fqoCalls = mocks.fetchQueryObservableWithInfo.mock.calls;
const fqoCalls = mocks.fetchConcastWithInfo.mock.calls;
expect(fqoCalls.length).toBe(2);
expect(fqoCalls[0][1].fetchPolicy).toEqual("cache-first");
expect(fqoCalls[1][1].fetchPolicy).toEqual("network-only");
Expand Down Expand Up @@ -1058,7 +1058,7 @@ describe("ObservableQuery", () => {
// FetchPolicy does not switch to cache-first after the first
// network request.
expect(observable.options.fetchPolicy).toBe("no-cache");
const fqoCalls = mocks.fetchQueryObservableWithInfo.mock.calls;
const fqoCalls = mocks.fetchConcastWithInfo.mock.calls;
expect(fqoCalls.length).toBe(2);
expect(fqoCalls[1][1].fetchPolicy).toBe("no-cache");

Expand Down

0 comments on commit 18cb2f3

Please sign in to comment.