Skip to content

Commit

Permalink
Fully remove usage of fetchBlockingPromise for now.
Browse files Browse the repository at this point in the history
  • Loading branch information
benjamn committed Apr 28, 2022
1 parent cf9a7ab commit da79d21
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 108 deletions.
71 changes: 0 additions & 71 deletions src/core/__tests__/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2421,75 +2421,4 @@ describe('ObservableQuery', () => {
error: reject,
});
});

describe("options.fetchBlockingPromise", () => {
itAsync("should not block future refetches", (resolve, reject) => {
const query: TypedDocumentNode<{
linkCount: number;
}> = gql`query Counter { linkCount }`;

let linkCount = 0;
const client = new ApolloClient({
cache: new InMemoryCache(),
link: new ApolloLink(request => new Observable(observer => {
if (request.operationName === "Counter") {
observer.next({
data: {
linkCount: ++linkCount,
},
});
observer.complete();
}
})),
});

const observable = client.watchQuery({
query,
notifyOnNetworkStatusChange: true,
});

subscribeAndCount(reject, observable, (count, result) => {
if (count === 1) {
expect(result.loading).toBe(false);
expect(result.data).toEqual({
linkCount: 1,
});

client.cache.writeQuery({
query,
data: {
linkCount: 1234,
},
});

} else if (count === 2) {
expect(result.loading).toBe(false);
expect(result.data).toEqual({
linkCount: 1234,
});

// Make sure refetching is not blocked by the fetchBlockingPromise.
return observable.refetch();

} else if (count === 3) {
expect(result.loading).toBe(true);
expect(result.networkStatus).toBe(NetworkStatus.refetch);
expect(result.data).toEqual({
linkCount: 1234,
});

} else if (count === 4) {
expect(result.loading).toBe(false);
expect(result.networkStatus).toBe(NetworkStatus.ready);
expect(result.data).toEqual({
linkCount: 2,
});

setTimeout(resolve, 20);
} else {
reject(`Too many results (${count}, ${JSON.stringify(result)})`);
}
});
});
});
});
2 changes: 1 addition & 1 deletion src/react/hoc/__tests__/queries/skip.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ describe('[queries] skip', () => {
expect(this.props.skip).toBe(false);
expect(this.props.data!.loading).toBe(true);
expect(this.props.data.allPeople).toEqual(data.allPeople);
expect(ranQuery).toBe(1);
expect(ranQuery).toBe(2);
break;
case 5:
expect(this.props.skip).toBe(false);
Expand Down
4 changes: 2 additions & 2 deletions src/react/hooks/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1078,8 +1078,8 @@ describe('useQuery Hook', () => {
expect(result.current.data).toEqual({ hello: 'from link' });
});

describe('options.fetchBlockingPromise', () => {
it("should prevent duplicate network requests in <React.StrictMode>", async () => {
describe('<React.StrictMode>', () => {
it("double-rendering should not trigger duplicate network requests", async () => {
const query: TypedDocumentNode<{
linkCount: number;
}> = gql`query Counter { linkCount }`;
Expand Down
38 changes: 4 additions & 34 deletions src/react/hooks/useQuery.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {
useCallback,
useContext,
useEffect,
useMemo,
useRef,
useState,
Expand Down Expand Up @@ -212,52 +211,23 @@ class InternalState<TData, TVariables> {
// allows us to depend on the referential stability of
// this.watchQueryOptions elsewhere.
const currentWatchQueryOptions = this.watchQueryOptions;
let resolveFetchBlockingPromise: undefined | ((result: boolean) => any);

if (!equal(watchQueryOptions, currentWatchQueryOptions)) {
this.watchQueryOptions = watchQueryOptions;
if (currentWatchQueryOptions && this.observable) {
// Though it might be tempting to postpone this setOptions call to the
// Though it might be tempting to postpone this reobserve call to the
// useEffect block, we need getCurrentResult to return an appropriate
// loading:true result synchronously (later within the same call to
// useQuery). Since we already have this.observable here (not true for
// the very first call to useQuery), we are not initiating any new
// subscriptions, though it does feel less than ideal that setOptions
// subscriptions, though it does feel less than ideal that reobserve
// (potentially) kicks off a network request (for example, when the
// variables have changed). To prevent any risk of premature/unwanted
// network traffic, we use a fetchBlockingPromise, which will only be
// unblocked once the useEffect has fired.
this.observable.reobserve(
watchQueryOptions,
void 0,
new Promise<boolean>(resolve => {
resolveFetchBlockingPromise = resolve;
}),
);

// variables have changed), which is technically a side-effect.
this.observable.reobserve(watchQueryOptions);
this.previousData = this.result?.data || this.previousData;
this.result = void 0;
}
}

if (resolveFetchBlockingPromise) {
if (this.renderPromises) {
// Since we're doing SSR, the useEffect callback will not be called, so
// we must unblock the fetchBlockingPromise now.
resolveFetchBlockingPromise(true);
} else {
// Otherwise, silently discard blocked fetches whose useEffect callbacks
// have not fired within 5 seconds (more than enough time to mount).
setTimeout(() => resolveFetchBlockingPromise!(false), 5000);
}
}

useEffect(() => {
if (resolveFetchBlockingPromise) {
resolveFetchBlockingPromise(true);
}
}, [resolveFetchBlockingPromise]);

// Make sure state.onCompleted and state.onError always reflect the latest
// options.onCompleted and options.onError callbacks provided to useQuery,
// since those functions are often recreated every time useQuery is called.
Expand Down

0 comments on commit da79d21

Please sign in to comment.