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.

Sign up for GitHub

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

Fix extra useQuery result frames #9599

Merged
merged 24 commits into from
Apr 18, 2022
Merged

Fix extra useQuery result frames #9599

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5a3f033
Added useQuery frame tests to show that there is an unneeded frame.
FritsvanCampen Mar 10, 2022
3d7fa34
Strengthen frame tests with expected NetworkStatus values.
benjamn Mar 22, 2022
7f98827
TODOs
benjamn Mar 21, 2022
6540b23
Call this.observable.setOptions when options change in useOptions.
benjamn Mar 22, 2022
a90922c
Fix lifecycle test extraneous results.
benjamn Mar 22, 2022
95a1749
Fix faulty skip tests.
benjamn Mar 22, 2022
9b03177
Improve mutation test timing robustness.
benjamn Apr 7, 2022
7e723db
Support WatchQueryOptions.fetchBlockingPromise.
benjamn Apr 14, 2022
7b80c93
Use fetchBlockingPromise when calling setOptions.
benjamn Apr 14, 2022
db0bd4b
Fix skip tests previously suffering from extraneous results.
benjamn Apr 14, 2022
85ae61f
Correct the frame test.
benjamn Apr 13, 2022
655fe74
Make the corrected frame test pass.
benjamn Apr 14, 2022
2101a72
Make other tests pass.
benjamn Apr 14, 2022
e94b545
Remove UNNEEDED_FRAME from useQuery tests.
benjamn Apr 14, 2022
d0265d4
Remove non-essential code (no tests fail).
benjamn Apr 14, 2022
a506feb
Use fetchBlockingPromise for initial useQuery request as well.
benjamn Apr 14, 2022
d047ab1
Decompose and reuse useUnblockFetchEffect helper function.
benjamn Apr 14, 2022
6033827
Silently discard blocked fetches after five seconds without useEffect.
benjamn Apr 14, 2022
7331572
Call this.observable.reobserve directly, instead of setOptions.
benjamn Apr 14, 2022
4ba569e
Bump bundlesize limit to 29.4kB (now 29.35kB).
benjamn Apr 14, 2022
b992e37
Reduce RenderPromises clutter in useQuery implementation.
benjamn Apr 15, 2022
c402dd4
Handle fetchBlockingPromise rejections more gracefully.
benjamn Apr 18, 2022
241f6ee
Basic tests of using options.fetchBlockingPromise directly.
benjamn Apr 18, 2022
58cab4e
Test fetchBlockingPromise prevents duplicate requests in <StrictMode>.
benjamn Apr 18, 2022
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
Prev Previous commit
Next Next commit
Make other tests pass.
  • Loading branch information
@benjamn
benjamn committed Apr 14, 2022
commit 2101a72e6be991e6ddcfa0948320ef21cc5ca896
19 changes: 9 additions & 10 deletions src/react/components/__tests__/client/Mutation.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1027,33 +1027,32 @@ describe('General Mutation testing', () => {
if (count === 0) {
// "first: 1" loading
expect(resultQuery.loading).toBe(true);
expect(resultQuery.data).toBeUndefined();
} else if (count === 1) {
// "first: 1" loaded
expect(resultQuery.loading).toBe(false);
expect(resultQuery.data).toEqual(peopleData1);
setTimeout(() => setVariables({ first: 2 }));
} else if (count === 2) {
expect(resultQuery.loading).toBe(false);
expect(resultQuery.data).toEqual(peopleData1);
} else if (count === 3) {
// "first: 2" loading
expect(resultQuery.loading).toBe(true);
} else if (count === 4) {
// "first: 2" loaded
expect(resultQuery.data).toBeUndefined();
} else if (count === 3) {
expect(resultQuery.loading).toBe(false);
expect(resultQuery.data).toEqual(peopleData2);
setTimeout(() => createTodo());
} else if (count === 5) {
} else if (count === 4) {
// mutation loading
expect(resultMutation.loading).toBe(true);
} else if (count === 6) {
} else if (count === 5) {
// mutation loaded
expect(resultMutation.loading).toBe(false);
} else if (count === 7) {
} else if (count === 6) {
// query refetched
expect(resultQuery.loading).toBe(false);
expect(resultMutation.loading).toBe(false);
expect(resultQuery.data).toEqual(peopleData3);
} else {
reject(`Too many renders (${count})`);
}
count++;
} catch (err) {
Expand All @@ -1074,7 +1073,7 @@ describe('General Mutation testing', () => {
);

waitFor(() => {
expect(count).toEqual(8);
expect(count).toEqual(7);
}).then(resolve, reject);
}));

Expand Down
62 changes: 33 additions & 29 deletions src/react/components/__tests__/client/Query.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1062,15 +1062,17 @@ describe('Query component', () => {
return (
<AllPeopleQuery query={query} variables={variables}>
{(result: any) => {
if (result.loading) {
return null;
}

try {
switch (count) {
case 0:
switch (++count) {
case 1:
expect(result.loading).toBe(true);
expect(result.data).toBeUndefined();
expect(variables).toEqual({ first: 1 });
break;
case 2:
expect(result.loading).toEqual(false);
expect(result.data).toEqual(data1);
expect(variables).toEqual({ first: 1 });
setTimeout(() => {
this.setState({
variables: {
Expand All @@ -1079,20 +1081,23 @@ describe('Query component', () => {
});
});
break;
case 1:
case 3:
expect(result.loading).toEqual(true);
expect(result.data).toBeUndefined();
expect(variables).toEqual({ first: 2 });
expect(result.data).toEqual(data1);
break;
case 2:
expect(variables).toEqual({ first: 2 });
case 4:
expect(result.loading).toEqual(false);
expect(result.data).toEqual(data2);
expect(variables).toEqual({ first: 2 });
break;
default:
reject(`Too many renders (${count})`);
}
} catch (error) {
reject(error);
}

count++;
return null;
}}
</AllPeopleQuery>
Expand All @@ -1106,7 +1111,7 @@ describe('Query component', () => {
</MockedProvider>
);

waitFor(() => expect(count).toBe(3)).then(resolve, reject);
waitFor(() => expect(count).toBe(4)).then(resolve, reject);
});

itAsync('if the query changes', (resolve, reject) => {
Expand Down Expand Up @@ -1232,27 +1237,27 @@ describe('Query component', () => {
<AllPeopleQuery query={query} variables={variables}>
{(result: any) => {
try {
switch (count) {
case 0:
switch (++count) {
case 1:
expect(result.loading).toBe(true);
expect(result.data).toBeUndefined();
expect(result.networkStatus).toBe(NetworkStatus.loading);
break;
case 1:
setTimeout(() => {
this.setState({ variables: { first: 2 } });
});
// fallthrough
case 2:
expect(result.loading).toBe(false);
expect(result.data).toEqual(data1);
expect(result.networkStatus).toBe(NetworkStatus.ready);
setTimeout(() => {
this.setState({ variables: { first: 2 } });
});
break;
case 3:
expect(result.loading).toBe(true);
expect(result.data).toBeUndefined();
expect(result.networkStatus).toBe(NetworkStatus.setVariables);
break;
case 4:
expect(result.loading).toBe(false);
expect(result.data).toEqual(data2);
expect(result.networkStatus).toBe(NetworkStatus.ready);
break;
Expand All @@ -1261,7 +1266,6 @@ describe('Query component', () => {
reject(err);
}

count++;
return null;
}}
</AllPeopleQuery>
Expand All @@ -1275,7 +1279,7 @@ describe('Query component', () => {
</MockedProvider>
);

waitFor(() => expect(count).toBe(5)).then(resolve, reject);
waitFor(() => expect(count).toBe(4)).then(resolve, reject);
});

itAsync('should update if a manual `refetch` is triggered after a state change', (resolve, reject) => {
Expand Down Expand Up @@ -1720,34 +1724,33 @@ describe('Query component', () => {
onCompleted={this.onCompleted}
>
{({ loading, data }: any) => {
switch (renderCount) {
case 0:
switch (++renderCount) {
case 1:
expect(loading).toBe(true);
expect(data).toBeUndefined();
break;
case 1:
case 2:
expect(loading).toBe(false);
expect(data).toEqual(data1);
break;
case 3:
expect(loading).toBe(true);
expect(data).toBeUndefined();
break;
case 4:
expect(loading).toBe(false);
expect(data).toEqual(data2);
setTimeout(() => {
this.setState({ variables: { first: 1 } });
});
case 5:
expect(loading).toBe(false);
expect(data).toEqual(data2);
break;
case 6:
case 5:
expect(loading).toBe(false);
expect(data).toEqual(data1);
break;
default:
reject(`Too many renders (${renderCount})`);
}
renderCount += 1;
return null;
}}
</AllPeopleQuery>
Expand All @@ -1762,6 +1765,7 @@ describe('Query component', () => {
);

waitFor(() => {
expect(renderCount).toBe(5);
expect(onCompletedCallCount).toBe(3);
}).then(resolve, reject);
});
Expand Down
22 changes: 12 additions & 10 deletions src/react/hoc/__tests__/queries/errors.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,18 +224,15 @@ describe('[queries] errors', () => {
try {
if (iteration === 1) {
// initial loading state is done, we have data
expect(props.data!.allPeople).toEqual(
data.allPeople
);
expect(props.data!.loading).toBe(false);
expect(props.data!.allPeople).toEqual(data.allPeople);
props.setVar(2);
} else if (iteration === 2) {
expect(props.data!.allPeople).toEqual(
data.allPeople
);
expect(props.data!.loading).toBe(true);
expect(props.data!.allPeople).toBeUndefined();
} else if (iteration === 3) {
// variables have changed, wee are loading again but also have data
expect(props.data!.loading).toBeTruthy();
} else if (iteration === 4) {
expect(props.data!.loading).toBe(false);
expect(props.data!.allPeople).toBeUndefined();
// the second request had an error!
expect(props.data!.error).toBeTruthy();
expect(props.data!.error!.networkError).toBeTruthy();
Expand All @@ -248,6 +245,8 @@ describe('[queries] errors', () => {
}
done = true;
});
} else {
reject(`Too many iterations (${iteration})`);
}
} catch (err) {
reject(err);
Expand All @@ -266,7 +265,10 @@ describe('[queries] errors', () => {
</ApolloProvider>
);

waitFor(() => expect(done).toBeTruthy()).then(resolve, reject);
waitFor(() => {
expect(done).toBeTruthy();
expect(iteration).toBe(3);
}).then(resolve, reject);
});
});

Expand Down
8 changes: 2 additions & 6 deletions src/react/hoc/__tests__/queries/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,14 +181,10 @@ describe('queries', () => {
expect(data!.variables).toEqual({ someId: 1 });
break;
case 2:
expect(data!.loading).toBe(true);
expect(data!.variables).toEqual({ someId: 1 });
break;
case 3:
expect(data!.loading).toBe(true);
expect(data!.variables).toEqual({ someId: 2 });
break;
case 4:
case 3:
expect(data!.loading).toBe(false);
expect(data!.variables).toEqual({ someId: 2 });
break;
Expand All @@ -211,7 +207,7 @@ describe('queries', () => {
);

waitFor(() => {
expect(count).toBe(4);
expect(count).toBe(3);
}).then(resolve, reject);
});

Expand Down
Loading