Query component uses hook #1519
Conversation
CameronGorrie
force-pushed
the
react-graphql-declarative-query
branch
2 times, most recently
from
3488a16
to
5881b46
Compare
CameronGorrie
force-pushed
the
react-graphql-declarative-query
branch
2 times, most recently
from
fa69544
to
ec214d2
Compare
CameronGorrie
force-pushed
the
react-graphql-declarative-query
branch
3 times, most recently
from
60c4306
to
13a8a23
Compare
CameronGorrie
force-pushed
the
react-graphql-declarative-query
branch
from
92e0e26
to
105ae94
Compare
| }: QueryComponentOptions<Data, Variables>) { | ||
| const opts = [options] as IfAllNullableKeys< | ||
| Variables, | ||
| [QueryHookOptions<Data, NoInfer<Variables>>?], |
There was a problem hiding this comment.
Should there be a ? in there?
There was a problem hiding this comment.
Yes it should. Chris added some context regarding the query options type here: #1519 (comment)
| const graphQL = createGraphQL({PetQuery: mockData}); | ||
| const renderPropSpy = jest.fn(() => null); | ||
|
|
||
| await mountWithGraphQL(<Query query={petQuery}>{renderPropSpy}</Query>, { |
There was a problem hiding this comment.
Nice! This is a great start. I'd also love to add a way to test the SSR issue that this component solves. But I don't think that is required for this PR.
There was a problem hiding this comment.
I could add a set of tests to ensure SSR behaviour. Do you feel these are implementation details of useQuery that are already tested though? https://github.com/Shopify/quilt/blob/master/packages/react-graphql/src/hooks/tests/query-ssr.test.tsx
There was a problem hiding this comment.
I was referring to the double call on ssr., but I think we have good coverage overall.
| @@ -25,7 +25,6 @@ | |||
| "homepage": "https://github.com/Shopify/quilt/blob/master/packages/react-graphql/README.md", | |||
| "dependencies": { | |||
| "@apollo/react-common": ">=3.0.0 <4.0.0", | |||
| "@apollo/react-components": ">=3.0.0 <4.0.0", | |||
| "@apollo/react-hooks": ">=3.0.0 <4.0.0", | |||
There was a problem hiding this comment.
Should this have been removed?
There was a problem hiding this comment.
Yes. Before this package was just re-exporting the Query component from Apollo which was imported from @apollo/react-components. Now that we're defining our own component the import can be safely removed.
Description
Fixes #1475
Client-side queries are being executed on the server when using the Apollo Query component re-exported from
react-graphql. This PR replaces the declarative component with our own implementation which uses the custom useQuery hook and handles client-only queries correctly.Type of change
react-graphqlPatch: Bug (non-breaking change which fixes an issue)Checklist