Skip to content

Commit b147cac

Browse files
phryneasphryneas
authored
drop fetchPolicy: "standby" pre-subscription loading state (#12631)
* prevent `fetchPolicy: standby` loading states * update tests * chore * adopt "keep old `previousResult` if data is equal" logic from #12597 * changesets * chores * add new test * `maybeDeepFreeze` `skipStandbyResult` * chores
1 parent 1bdf489 commit b147cac

File tree

10 files changed

+109
-68
lines changed

10 files changed

+109
-68
lines changed

‎.api-reports/api-report-react.api.md‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,8 +1366,6 @@ export namespace useQuery {
13661366
export namespace useQuery {
13671367
var // (undocumented)
13681368
ssrDisabledResult: ApolloQueryResult_2<any>;
1369-
var // (undocumented)
1370-
skipStandbyResult: ApolloQueryResult_2<any>;
13711369
}
13721370

13731371
// @public

‎.changeset/beige-spiders-hope.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@apollo/client": patch
3+
---
4+
5+
When updating `skip` from `false` to `true` in `useQuery`, retain `data` if it is available rather than setting it to `undefined`.

‎.changeset/sour-colts-tell.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@apollo/client": major
3+
---
4+
5+
`ObservableQuery` will now return a `loading: false` state for `fetchPolicy` `standby`, even before subscription

‎.changeset/strange-walls-march.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@apollo/client": patch
3+
---
4+
5+
The `error` property is no longer present when `skip` is `true` in `useQuery`.

‎.size-limits.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 43088,
3-
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 38183,
4-
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 33033,
5-
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 27456
2+
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 43129,
3+
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 38142,
4+
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 33072,
5+
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 27518
66
}

‎src/core/ObservableQuery.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,12 @@ export class ObservableQuery<
513513
loading: true,
514514
networkStatus: NetworkStatus.loading,
515515
};
516+
case "standby":
517+
return {
518+
...defaultResult,
519+
loading: false,
520+
networkStatus: NetworkStatus.ready,
521+
};
516522

517523
default:
518524
return defaultResult;

‎src/core/__tests__/ObservableQuery.ts‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4036,9 +4036,8 @@ describe("ObservableQuery", () => {
40364036
true,
40374037
{
40384038
resultBeforeSubscribe: {
4039-
// TODO: should this be done from the start?
40404039
currentResult: {
4041-
...loadingStates.loading,
4040+
...loadingStates.done,
40424041
data: undefined,
40434042
partial: true,
40444043
},
@@ -4078,9 +4077,8 @@ describe("ObservableQuery", () => {
40784077
false,
40794078
{
40804079
resultBeforeSubscribe: {
4081-
// TODO: should this be done from the start?
40824080
currentResult: {
4083-
...loadingStates.loading,
4081+
...loadingStates.done,
40844082
data: undefined,
40854083
partial: true,
40864084
},
@@ -4120,9 +4118,8 @@ describe("ObservableQuery", () => {
41204118
true,
41214119
{
41224120
resultBeforeSubscribe: {
4123-
// TODO: should this be done from the start?
41244121
currentResult: {
4125-
...loadingStates.loading,
4122+
...loadingStates.done,
41264123
data: undefined,
41274124
partial: true,
41284125
},
@@ -4162,9 +4159,8 @@ describe("ObservableQuery", () => {
41624159
false,
41634160
{
41644161
resultBeforeSubscribe: {
4165-
// TODO: should this be done from the start?
41664162
currentResult: {
4167-
...loadingStates.loading,
4163+
...loadingStates.done,
41684164
data: undefined,
41694165
partial: true,
41704166
},

‎src/react/hooks/__tests__/useQuery.test.tsx‎

Lines changed: 59 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,7 +1558,6 @@ describe("useQuery Hook", () => {
15581558

15591559
await expect(takeSnapshot()).resolves.toStrictEqualTyped({
15601560
data: undefined,
1561-
error: undefined,
15621561
loading: false,
15631562
networkStatus: NetworkStatus.ready,
15641563
previousData: undefined,
@@ -1584,7 +1583,6 @@ describe("useQuery Hook", () => {
15841583

15851584
await expect(takeSnapshot()).resolves.toStrictEqualTyped({
15861585
data: undefined,
1587-
error: undefined,
15881586
loading: false,
15891587
networkStatus: NetworkStatus.ready,
15901588
previousData: { user: { __typename: "User", id: 1, name: "User 1" } },
@@ -1936,7 +1934,7 @@ describe("useQuery Hook", () => {
19361934

19371935
const cache = new InMemoryCache();
19381936
using _disabledAct = disableActEnvironment();
1939-
const { takeSnapshot, rerender } = await renderHookToSnapshotStream(
1937+
const renderStream = await renderHookToSnapshotStream(
19401938
({ skip }) => useQuery(query, { pollInterval: 80, skip }),
19411939
{
19421940
initialProps: { skip: false },
@@ -1951,7 +1949,7 @@ describe("useQuery Hook", () => {
19511949
),
19521950
}
19531951
);
1954-
1952+
const { takeSnapshot, rerender } = renderStream;
19551953
{
19561954
const result = await takeSnapshot();
19571955

@@ -1978,20 +1976,9 @@ describe("useQuery Hook", () => {
19781976

19791977
await rerender({ skip: true });
19801978

1981-
{
1982-
const result = await takeSnapshot();
1983-
1984-
expect(result).toStrictEqualTyped({
1985-
// TODO: wut?
1986-
data: undefined,
1987-
// TODO: wut?
1988-
error: undefined,
1989-
loading: false,
1990-
networkStatus: NetworkStatus.ready,
1991-
previousData: { hello: "world 1" },
1992-
variables: {},
1993-
});
1994-
}
1979+
await expect(renderStream).toRerenderWithSimilarSnapshot({
1980+
/* equal result */
1981+
});
19951982

19961983
await expect(takeSnapshot).not.toRerender({ timeout: 100 });
19971984

@@ -2004,7 +1991,7 @@ describe("useQuery Hook", () => {
20041991
data: { hello: "world 1" },
20051992
loading: false,
20061993
networkStatus: NetworkStatus.ready,
2007-
previousData: { hello: "world 1" },
1994+
previousData: undefined,
20081995
variables: {},
20091996
});
20101997
}
@@ -7120,7 +7107,6 @@ describe("useQuery Hook", () => {
71207107

71217108
await expect(takeSnapshot()).resolves.toStrictEqualTyped({
71227109
data: undefined,
7123-
error: undefined,
71247110
loading: false,
71257111
networkStatus: NetworkStatus.ready,
71267112
previousData: undefined,
@@ -7189,7 +7175,6 @@ describe("useQuery Hook", () => {
71897175

71907176
await expect(takeSnapshot()).resolves.toStrictEqualTyped({
71917177
data: undefined,
7192-
error: undefined,
71937178
loading: false,
71947179
networkStatus: NetworkStatus.ready,
71957180
previousData: { hello: "world" },
@@ -7245,7 +7230,6 @@ describe("useQuery Hook", () => {
72457230

72467231
await expect(takeSnapshot()).resolves.toStrictEqualTyped({
72477232
data: undefined,
7248-
error: undefined,
72497233
loading: false,
72507234
networkStatus: NetworkStatus.ready,
72517235
previousData: undefined,
@@ -7273,6 +7257,59 @@ describe("useQuery Hook", () => {
72737257
await expect(takeSnapshot).not.toRerender();
72747258
});
72757259

7260+
it("should not automatically set `data` to `undefined` when `skip` becomes `true`", async () => {
7261+
const query = gql`
7262+
{
7263+
hello
7264+
}
7265+
`;
7266+
const mocks = [
7267+
{
7268+
request: { query },
7269+
result: { data: { hello: "world" } },
7270+
},
7271+
];
7272+
7273+
using _disabledAct = disableActEnvironment();
7274+
const { takeSnapshot, rerender } = await renderHookToSnapshotStream(
7275+
({ skip }) => useQuery(query, { skip }),
7276+
{
7277+
wrapper: ({ children }) => (
7278+
<MockedProvider mocks={mocks}>{children}</MockedProvider>
7279+
),
7280+
initialProps: { skip: false },
7281+
}
7282+
);
7283+
7284+
await expect(takeSnapshot()).resolves.toStrictEqualTyped({
7285+
data: undefined,
7286+
loading: true,
7287+
networkStatus: NetworkStatus.loading,
7288+
previousData: undefined,
7289+
variables: {},
7290+
});
7291+
7292+
await expect(takeSnapshot()).resolves.toStrictEqualTyped({
7293+
data: { hello: "world" },
7294+
loading: false,
7295+
networkStatus: NetworkStatus.ready,
7296+
previousData: undefined,
7297+
variables: {},
7298+
});
7299+
7300+
await rerender({ skip: true });
7301+
7302+
await expect(takeSnapshot()).resolves.toStrictEqualTyped({
7303+
data: { hello: "world" },
7304+
loading: false,
7305+
networkStatus: NetworkStatus.ready,
7306+
previousData: undefined,
7307+
variables: {},
7308+
});
7309+
7310+
await expect(takeSnapshot).not.toRerender();
7311+
});
7312+
72767313
// Amusingly, #8270 thinks this is a bug, but #9101 thinks this is not.
72777314
it("should refetch when skip is true", async () => {
72787315
const query = gql`
@@ -7305,7 +7342,6 @@ describe("useQuery Hook", () => {
73057342

73067343
await expect(takeSnapshot()).resolves.toStrictEqualTyped({
73077344
data: undefined,
7308-
error: undefined,
73097345
loading: false,
73107346
networkStatus: NetworkStatus.ready,
73117347
previousData: undefined,
@@ -7367,7 +7403,6 @@ describe("useQuery Hook", () => {
73677403

73687404
await expect(takeSnapshot()).resolves.toStrictEqualTyped({
73697405
data: undefined,
7370-
error: undefined,
73717406
loading: false,
73727407
networkStatus: NetworkStatus.ready,
73737408
previousData: undefined,

‎src/react/hooks/useQuery.ts‎

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -306,27 +306,12 @@ function useQuery_<TData, TVariables extends OperationVariables>(
306306
watchQueryOptions
307307
);
308308

309-
const ssrDisabledOverride = useSyncExternalStore(
309+
const resultOverride = useSyncExternalStore(
310310
() => () => {},
311311
() => void 0,
312312
() => (ssr === false ? useQuery.ssrDisabledResult : void 0)
313313
);
314314

315-
const resultOverride =
316-
skip || watchQueryOptions.fetchPolicy === "standby" ?
317-
// When skipping a query (ie. we're not querying for data but still want to
318-
// render children), make sure the `data` is cleared out and `loading` is
319-
// set to `false` (since we aren't loading anything).
320-
//
321-
// NOTE: We no longer think this is the correct behavior. Skipping should
322-
// not automatically set `data` to `undefined`, but instead leave the
323-
// previous data in place. In other words, skipping should not mandate that
324-
// previously received data is all of a sudden removed. Unfortunately,
325-
// changing this is breaking, so we'll have to wait until Apollo Client 4.0
326-
// to address this.
327-
useQuery.skipStandbyResult
328-
: ssrDisabledOverride;
329-
330315
const result = useResultSubscription<TData, TVariables>(
331316
observable,
332317
resultData,
@@ -461,9 +446,13 @@ function useResubscribeIfNecessary<
461446
// Make sure getCurrentResult returns a fresh ApolloQueryResult<TData>,
462447
// but save the current data as this.previousData, just like setResult
463448
// usually does.
464-
resultData.previousData =
465-
resultData.current.data || resultData.previousData;
466-
resultData.current = observable.getCurrentResult();
449+
const result = observable.getCurrentResult();
450+
451+
if (!equal(result.data, resultData.current.data)) {
452+
resultData.previousData =
453+
resultData.current.data || resultData.previousData;
454+
}
455+
resultData.current = result;
467456
}
468457
observable[lastWatchOptions] = watchQueryOptions;
469458
}
@@ -475,11 +464,3 @@ useQuery.ssrDisabledResult = maybeDeepFreeze({
475464
networkStatus: NetworkStatus.loading,
476465
partial: true,
477466
}) satisfies ApolloQueryResult<any> as ApolloQueryResult<any>;
478-
479-
useQuery.skipStandbyResult = maybeDeepFreeze({
480-
loading: false,
481-
data: void 0 as any,
482-
error: void 0,
483-
networkStatus: NetworkStatus.ready,
484-
partial: true,
485-
}) satisfies ApolloQueryResult<any> as ApolloQueryResult<any>;

‎src/react/ssr/useSSRQuery.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,20 @@
11
import type { DocumentNode } from "graphql";
22

3-
import type { ObservableQuery } from "@apollo/client";
3+
import type { ApolloQueryResult, ObservableQuery } from "@apollo/client";
4+
import { NetworkStatus } from "@apollo/client";
45
import { useApolloClient, useQuery } from "@apollo/client/react";
6+
import { maybeDeepFreeze } from "@apollo/client/utilities/internal";
57

68
import type { PrerenderStaticInternalContext } from "./prerenderStatic.js";
79

10+
const skipStandbyResult: ApolloQueryResult<any> = maybeDeepFreeze({
11+
loading: false,
12+
data: void 0 as any,
13+
error: void 0,
14+
networkStatus: NetworkStatus.ready,
15+
partial: true,
16+
});
17+
818
export const useSSRQuery = function (
919
this: PrerenderStaticInternalContext,
1020
query: DocumentNode,
@@ -30,10 +40,10 @@ export const useSSRQuery = function (
3040
previousData: undefined,
3141
};
3242

33-
if (options.skip) {
43+
if (options.skip || options.fetchPolicy === "standby") {
3444
return withoutObservableAccess({
3545
...baseResult,
36-
...useQuery.skipStandbyResult,
46+
...skipStandbyResult,
3747
});
3848
}
3949
if (options.ssr === false) {

0 commit comments

Comments
 (0)