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
Removing onCompleted and onError from the useQuery types #2645
base: main
Are you sure you want to change the base?
Conversation
230ce1c
to
d00962d
Compare
Change looks reasonable though I'm concerned that folks might have been using these props without realising they do nothing, and thus removing them from the type will introduce a bunch of type errors in web. Could you please cut a snapshot and test if using it in web has an effect on running type-check? |
/snapit |
🫰✨ Thanks @BPScott! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/react-graphql@0.0.0-snapshot-20230503131115 yarn add @shopify/react-graphql-universal-provider@0.0.0-snapshot-20230503131115 |
I see some instances |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the impact in web is pretty light and if we release this fixing web's CI would be pretty quick. I'm ok with claiming this is a patch release and we have to fix those places in web.
Code changes look good, I've added a suggestion for how to format the changeset so something that would feel a bit nicer to read as a changelog entry.
Alex, can you own getting this updated within web, so we we don't have to do the tidying as part of some future update?
db126a3
to
22739ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Can you own getting this updated in web (I've moved onto checkout things).
22739ad
to
20789f2
Compare
20789f2
to
2e35533
Compare
Description
Our custom implementation of
useQuery
and<Query />
doesn't supportonCompleted
andonError
.This PR update the types to reflect the difference between our API and Apollo API.