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

Jump to bottom

Adjust relative timing of useFragment and useQuery #11083

Merged
merged 10 commits into from
Jul 28, 2023
Merged

Adjust relative timing of useFragment and useQuery #11083

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
11810dc
work on useFragment timing
phryneas Jul 19, 2023
473476f
changeset
phryneas Jul 19, 2023
e60068e
all tests green
phryneas Jul 24, 2023
e971a18
remove most `IS_REACT_18` checks
phryneas Jul 24, 2023
6b409ff
Merge remote-tracking branch 'origin/release-3.8' into pr/useFragment…
phryneas Jul 24, 2023
2dde3e4
changeset
phryneas Jul 24, 2023
30699cc
Update src/react/hooks/useQuery.ts
phryneas Jul 24, 2023
1643940
Update .changeset/honest-ads-act.md
phryneas Jul 28, 2023
4594974
feedback from code review
phryneas Jul 28, 2023
d5db4c4
Merge remote-tracking branch 'origin/release-3.8' into pr/useFragment…
phryneas Jul 28, 2023
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
5 changes: 5 additions & 0 deletions .changeset/honest-ads-act.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@apollo/client': patch
---

This needs a changeset for a PR release
27 changes: 15 additions & 12 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 @@ -17,8 +17,6 @@ import {
import { Query } from '../../Query';
import { Mutation } from '../../Mutation';

const IS_REACT_18 = React.version.startsWith('18');

const mutation = gql`
mutation createTodo($text: String!) {
createTodo {
Expand Down Expand Up @@ -978,7 +976,7 @@ describe('General Mutation testing', () => {
);
});

it('allows a refetchQueries prop as string and variables have updated', async () => new Promise((resolve, reject) => {
it('allows a refetchQueries prop as string and variables have updated', async () => {
const query = gql`
query people($first: Int) {
allPeople(first: $first) {
Expand Down Expand Up @@ -1026,6 +1024,7 @@ describe('General Mutation testing', () => {
const refetchQueries = ['people'];

let count = 0;
let testFailures: any[] = [];

const Component: React.FC<PropsWithChildren<PropsWithChildren<any>>> = props => {
const [variables, setVariables] = useState(props.variables);
Expand Down Expand Up @@ -1055,19 +1054,20 @@ describe('General Mutation testing', () => {
// mutation loading
expect(resultMutation.loading).toBe(true);
} else if (count === 5) {
// mutation loaded
expect(resultMutation.loading).toBe(false);
// query refetched or mutation loaded
// or both finished batched together
// hard to make assumptions here
} else if (count === 6) {
// query refetched
// both loaded
expect(resultQuery.loading).toBe(false);
expect(resultMutation.loading).toBe(false);
expect(resultQuery.data).toEqual(peopleData3);
} else {
reject(`Too many renders (${count})`);
throw new Error(`Too many renders (${count})`);
}
count++;
} catch (err) {
reject(err);
testFailures.push(err);
}
return null;
}}
Expand All @@ -1083,10 +1083,13 @@ describe('General Mutation testing', () => {
</MockedProvider>
);

waitFor(() => {
expect(count).toEqual(IS_REACT_18 ? 6 : 7);
}).then(resolve, reject);
}));
await waitFor(() => {
if (testFailures.length > 0) {
throw testFailures[0];
}
expect(count).toEqual(7);
});
});

it('allows refetchQueries to be passed to the mutate function', () => new Promise((resolve, reject) => {
const query = gql`
Expand Down
57 changes: 15 additions & 42 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 @@ -11,8 +11,6 @@ import { ApolloProvider } from '../../../context';
import { itAsync, MockedProvider, mockSingleLink } from '../../../../testing';
import { Query } from '../../Query';

const IS_REACT_18 = React.version.startsWith('18');

const allPeopleQuery: DocumentNode = gql`
query people {
allPeople(first: 1) {
Expand Down Expand Up @@ -293,11 +291,7 @@ describe('Query component', () => {
}
if (count === 3) {
// second data
if (IS_REACT_18) {
expect(data).toEqual(data3);
} else {
expect(data).toEqual(data2);
}
expect(data).toEqual(data2);
}
if (count === 5) {
// third data
Expand Down Expand Up @@ -338,11 +332,7 @@ describe('Query component', () => {
);

waitFor(() => {
if (IS_REACT_18) {
expect(count).toBe(4);
} else {
expect(count).toBe(6);
}
expect(count).toBe(6);
}).then(resolve, reject);
});

Expand Down Expand Up @@ -688,11 +678,7 @@ describe('Query component', () => {
});
}
if (count === 2) {
if (IS_REACT_18) {
expect(result.loading).toBeFalsy();
} else {
expect(result.loading).toBeTruthy();
}
expect(result.loading).toBeTruthy();
}
if (count === 3) {
expect(result.loading).toBeFalsy();
Expand All @@ -714,11 +700,7 @@ describe('Query component', () => {
);

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

Expand Down Expand Up @@ -1378,11 +1360,7 @@ describe('Query component', () => {
break;
case 3:
// Second response loading
if (IS_REACT_18) {
expect(props.loading).toBe(false);
} else {
expect(props.loading).toBe(true);
}
expect(props.loading).toBe(true);
break;
case 4:
// Second response received, fire another refetch
Expand Down Expand Up @@ -1416,11 +1394,7 @@ describe('Query component', () => {
render(<SomeComponent />);

waitFor(() => {
if (IS_REACT_18) {
expect(count).toBe(4)
} else {
expect(count).toBe(7)
}
expect(count).toBe(7)
}).then(resolve, reject);
});
});
Expand Down Expand Up @@ -1502,6 +1476,7 @@ describe('Query component', () => {
});

let count = 0;
let testFailures: any[] = [];
const noop = () => null;

const AllPeopleQuery2 = Query;
Expand Down Expand Up @@ -1530,11 +1505,7 @@ describe('Query component', () => {
break;
case 2:
// Waiting for the second result to load
if (IS_REACT_18) {
expect(result.loading).toBe(false);
} else {
expect(result.loading).toBe(true);
}
expect(result.loading).toBe(true);
break;
case 3:
setTimeout(() => {
Expand All @@ -1561,7 +1532,10 @@ describe('Query component', () => {
throw new Error('Unexpected fall through');
}
} catch (e) {
fail(e);
// if we throw the error inside the component,
// we will get more rerenders in the test, but the `expect` error
// might not propagate anyways
testFailures.push(e);
}
return null;
}}
Expand All @@ -1576,11 +1550,10 @@ describe('Query component', () => {
);

await waitFor(() => {
if (IS_REACT_18) {
expect(count).toBe(3)
} else {
expect(count).toBe(6)
if (testFailures.length > 0) {
throw testFailures[0];
}
expect(count).toBe(6);
});
});

Expand Down
18 changes: 12 additions & 6 deletions src/react/components/__tests__/client/Subscription.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,14 +415,15 @@ itAsync('renders an error', (resolve, reject) => {
});

describe('should update', () => {
itAsync('if the client changes', (resolve, reject) => {
it('if the client changes', async () => {
const link2 = new MockSubscriptionLink();
const client2 = new ApolloClient({
link: link2,
cache: new Cache({ addTypename: false })
});

let count = 0;
let testFailures: any[] = [];

class Component extends React.Component {
state = {
Expand All @@ -436,7 +437,7 @@ describe('should update', () => {
{(result: any) => {
const { loading, data } = result;
try {
switch (count) {
switch (count++) {
case 0:
expect(loading).toBeTruthy();
expect(data).toBeUndefined();
Expand Down Expand Up @@ -465,12 +466,12 @@ describe('should update', () => {
expect(loading).toBeFalsy();
expect(data).toEqual(results[1].result.data);
break;
default:
throw new Error("too many rerenders");
}
} catch (error) {
reject(error);
testFailures.push(error);
}

count++;
return null;
}}
</Subscription>
Expand All @@ -483,7 +484,12 @@ describe('should update', () => {

link.simulateResult(results[0]);

waitFor(() => expect(count).toBe(5)).then(resolve, reject);
await waitFor(() => {
if (testFailures.length > 0) {
throw testFailures[0];
}
expect(count).toBe(5);
});
});

itAsync('if the query changes', (resolve, reject) => {
Expand Down
29 changes: 3 additions & 26 deletions src/react/hoc/__tests__/mutations/queries.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import {
import { graphql } from '../../graphql';
import { ChildProps } from '../../types';

const IS_REACT_18 = React.version.startsWith('18');

describe('graphql(mutation) query integration', () => {
itAsync('allows for passing optimisticResponse for a mutation', (resolve, reject) => {
const query: DocumentNode = gql`
Expand Down Expand Up @@ -194,15 +192,6 @@ describe('graphql(mutation) query integration', () => {
mutationData.createTodo
]);
break;
case 3:
if (IS_REACT_18) {
expect(this.props.data.todo_list.tasks).toEqual([
mutationData.createTodo
]);
} else {
reject(`too many renders (${renderCount})`);
}
break;
default:
reject(`too many renders (${renderCount})`);
}
Expand All @@ -220,11 +209,7 @@ describe('graphql(mutation) query integration', () => {
);

waitFor(() => {
if (IS_REACT_18) {
expect(renderCount).toBe(3);
} else {
expect(renderCount).toBe(2);
}
expect(renderCount).toBe(2);
}).then(resolve, reject);
});

Expand Down Expand Up @@ -325,11 +310,7 @@ describe('graphql(mutation) query integration', () => {
class extends React.Component<ChildProps<Variables, Data>> {
render() {
if (count === 1) {
if (IS_REACT_18) {
expect(this.props.data!.mini).toEqual(mutationData.mini);
} else {
expect(this.props.data!.mini).toEqual(queryData.mini);
}
expect(this.props.data!.mini).toEqual(queryData.mini);
}
if (count === 2) {
expect(this.props.data!.mini).toEqual(
Expand All @@ -354,11 +335,7 @@ describe('graphql(mutation) query integration', () => {
);

waitFor(() => {
if (IS_REACT_18) {
expect(count).toBe(2);
} else {
expect(count).toBe(3);
}
expect(count).toBe(3);
}).then(resolve, reject);
});

Expand Down
Loading
Loading