Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add test case that reproduces bug in issue #12096 #12071

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ export class ObservableQuery<
newResult: ApolloQueryResult<TData>,
variables?: TVariables
) {
console.log({ lastResult: this.last, newResult });
if (!this.last) {
return true;
}
Expand All @@ -332,6 +333,12 @@ export class ObservableQuery<
!equalByQuery(this.query, this.last.result, newResult, this.variables)
: !equal(this.last.result, newResult);

console.log({
resultIsDifferent,
variablesAreDifferent:
variables && !equal(this.last.variables, variables),
});

return (
resultIsDifferent || (variables && !equal(this.last.variables, variables))
);
Expand Down Expand Up @@ -969,6 +976,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
next: (result) => {
if (equal(this.variables, variables)) {
finishWaitingForOwnResult();
console.log("report result");
this.reportResult(result, variables);
}
},
Expand Down Expand Up @@ -1055,11 +1063,25 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
) {
const lastError = this.getLastError();
const isDifferent = this.isDifferentFromLastResult(result, variables);
console.log({ isDifferent });
// Update the last result even when isDifferentFromLastResult returns false,
// because the query may be using the @nonreactive directive, and we want to
// save the the latest version of any nonreactive subtrees (in case
// getCurrentResult is called), even though we skip broadcasting changes.
if (lastError || !result.partial || this.options.returnPartialData) {
console.log({
result,
lastError,
partial: !result.partial,
returnPartialData: this.options.returnPartialData,
});
const resultDataKeys: string[] | undefined =
result && result.data && Object.keys(result.data);
if (
lastError ||
!result.partial ||
this.options.returnPartialData ||
resultDataKeys?.length === 0
) {
this.updateLastResult(result, variables);
}
if (lastError || isDifferent) {
Expand Down
1 change: 1 addition & 0 deletions src/core/QueryInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ export class QueryInfo {
this.getDiffOptions(options.variables)
);
} else if (cacheWriteBehavior !== CacheWriteBehavior.FORBID) {
console.log("inside here");
if (shouldWriteResult(result, options.errorPolicy)) {
// Using a transaction here so we have a chance to read the result
// back from the cache before the watch callback fires as a result
Expand Down
7 changes: 4 additions & 3 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,7 @@ export class QueryManager<TStore> {
return fromData(void 0);
}

console.log("return from data", { data });
return fromData(data);
};

Expand Down Expand Up @@ -1613,19 +1614,19 @@ export class QueryManager<TStore> {
typeof oldNetworkStatus === "number" &&
oldNetworkStatus !== networkStatus &&
isNetworkRequestInFlight(networkStatus);

console.log({ fetchPolicy });
switch (fetchPolicy) {
default:
case "cache-first": {
const diff = readCache();

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

console.log({ returnPartialData, shouldNotify });
if (returnPartialData || shouldNotify) {
return {
fromLink: true,
Expand Down
106 changes: 106 additions & 0 deletions src/core/__tests__/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2403,6 +2403,112 @@ describe("ObservableQuery", () => {
}
);

it.only("bug: observable stuck on loading: true when notifyOnNetworkStatusChange is true and refetched data does not change with returnPartialData: true", async () => {
const queryOptions = {
query: gql`
query {
value
}
`,
returnPartialData: true,
notifyOnNetworkStatusChange: true,
};

const client = new ApolloClient({
link: mockSingleLink(
{ request: queryOptions, result: { data: { value: 1 } } },
{ request: queryOptions, result: { data: { value: 1 } } }
).setOnError((error) => {
throw error;
}),
cache: new InMemoryCache(),
});

const obs = client.watchQuery(queryOptions);

const stream = new ObservableStream(obs);

{
const result = await stream.takeNext();
expect(result.data).toEqual({});
expect(result.loading).toBe(true);
}

{
const result = await stream.takeNext();
expect(result.data).toEqual({ value: 1 });
expect(result.loading).toBe(false);
}

client.cache.modify({
fields: {
value: (_, { DELETE }) => DELETE,
},
});

{
const result = await stream.takeNext();
expect(result.data).toEqual({});
expect(result.loading).toBe(true);
}

{
const result = await stream.takeNext();
expect(result.data).toEqual({ value: 1 });
expect(result.loading).toBe(false);
}
});

// reproduction of https://github.com/apollographql/apollo-client/issues/12069
it.only("bug: observable stuck on loading: true when notifyOnNetworkStatusChange is true and refetched data does not change", async () => {
const queryOptions = {
query: gql`
query {
value
}
`,
notifyOnNetworkStatusChange: true,
};

const client = new ApolloClient({
link: mockSingleLink(
{ request: queryOptions, result: { data: { value: 1 } } },
{ request: queryOptions, result: { data: { value: 1 } } }
).setOnError((error) => {
throw error;
}),
cache: new InMemoryCache(),
});

const obs = client.watchQuery(queryOptions);

const stream = new ObservableStream(obs);

{
const result = await stream.takeNext();
expect(result.data).toEqual({ value: 1 });
expect(result.loading).toBe(false);
}

client.cache.modify({
fields: {
value: (_, { DELETE }) => DELETE,
},
});

{
const result = await stream.takeNext();
expect(result.data).toEqual({});
expect(result.loading).toBe(true);
}

{
const result = await stream.takeNext();
expect(result.data).toEqual({ value: 1 });
expect(result.loading).toBe(false);
}
});

it("handles multiple calls to getCurrentResult without losing data", async () => {
const query = gql`
{
Expand Down
Loading