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

Fix previousData lost with useLazyQuery and variable change #8459

Conversation

grnnja
Copy link

@grnnja grnnja commented Jul 6, 2021

This is a failing test for #7396. I based the new test on:

itAsync('should persist previous data when a query is re-run', (resolve, reject) => {

The component in the new test rerenders an extra time between rerender 4 and rerender 5, where loading is still true, but it changes both data and previousData to undefined, which is the problem in #7396.

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

@grnnja grnnja changed the title add useLazyQuery test for persisting data on var change Fix previousData lost with useLazyQuery and variable change Jul 6, 2021
@brainkim brainkim self-requested a review July 6, 2021 23:16
@brainkim brainkim added this to the July 2021 milestone Jul 6, 2021
@brainkim
Copy link
Contributor

brainkim commented Jul 6, 2021

Thanks for this! If you’re planning on looking into this, I’m happy to help in any way, and if you want a maintainer to expedite a fix for the related bug, this is best way to do that! Either way, ping me and let me know!

@grnnja
Copy link
Author

grnnja commented Jul 6, 2021

Hey @brainkim! I'm not too sure what the intended / correct behavior would look like, so perhaps its best for a maintainer to handle this.

@brainkim
Copy link
Contributor

brainkim commented Jul 13, 2021

@grnnja I took a look at this and wrote a comment on the original issue. If you use the refetch() function instead of the execute() function, it seems like you get the behavior you’re looking for (

itAsync('should persist previous data when a query is re-run and variable changes', (resolve, reject) => {
const CAR_QUERY_BY_ID = gql`
query($id: Int) {
car(id: $id) {
make
model
}
}
`;
const data1 = {
car: {
make: 'Audi',
model: 'A4',
__typename: 'Car',
},
};
const data2 = {
car: {
make: 'Audi',
model: 'RS8',
__typename: 'Car',
},
};
const mocks = [
{
request: { query: CAR_QUERY_BY_ID, variables: { id: 1 } },
result: { data: data1 },
},
{
request: { query: CAR_QUERY_BY_ID, variables: { id: 2 } },
result: { data: data2 },
},
];
let renderCount = 0;
function App() {
const [execute, { loading, data, previousData, refetch }] = useLazyQuery(
CAR_QUERY_BY_ID,
{ notifyOnNetworkStatusChange: true },
);
refetch;
switch (++renderCount) {
case 1:
expect(loading).toBe(false);
expect(data).toBeUndefined();
expect(previousData).toBeUndefined();
setTimeout(() => execute({ variables: { id: 1 }}));
break;
case 2:
expect(loading).toBe(true);
expect(data).toBeUndefined();
expect(previousData).toBeUndefined();
break;
case 3:
expect(loading).toBe(false);
expect(data).toEqual(data1);
expect(previousData).toBeUndefined();
setTimeout(() => refetch!({ id: 2 }));
break;
case 4:
expect(loading).toBe(true);
expect(data).toEqual(undefined);
expect(previousData).toEqual(data1);
break;
case 5:
expect(loading).toBe(false);
expect(data).toEqual(data2);
expect(previousData).toEqual(data1);
break;
default: // Do nothing
}
return null;
}
render(
<MockedProvider mocks={mocks}>
<App />
</MockedProvider>
);
return wait(() => {
expect(renderCount).toBe(5);
}).then(resolve, reject);
).

@grnnja
Copy link
Author

grnnja commented Jul 13, 2021

@brainkim Thanks for clearing this up, I didn't realize that refetch() behaved differently from execute().

@grnnja grnnja closed this Jul 13, 2021
@hwillson hwillson removed this from the MM-2021-07 milestone Jul 29, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants