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

feat : [typescript-react-apollo] support useSuspenseQuery #392

Merged
merged 14 commits into from
Oct 25, 2023

Conversation

Hal-ang
Copy link
Contributor

@Hal-ang Hal-ang commented Sep 10, 2023

Description

Implemented useSuspenseQuery, which was added in apollo 3.8.0 version.

Related #388

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as
    expected)
  • This change requires a documentation update

Screenshots/Sandbox (if appropriate/relevant):

Adding links to sandbox or providing screenshots can help us understand more about this PR and take
action on it as appropriate

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can
reproduce. Please also list any relevant details for your test configuration

  • I ran codegen with the changes applied to the project that is actually using the library, and confirmed that useSuspenseQuery was working properly. (with patch-package)
  • passed yarn run test after update snapshot file

Test Environment:

Checklist:

  • I have followed the
    CONTRIBUTING doc and the
    style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@changeset-bot
Copy link

changeset-bot bot commented Sep 10, 2023

🦋 Changeset detected

Latest commit: 38d320a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@graphql-codegen/typescript-react-apollo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Hal-ang Hal-ang changed the title Feat/use suspense query feat(apollo) - support useSuspenseQuery Sep 10, 2023
@Hal-ang Hal-ang changed the title feat(apollo) - support useSuspenseQuery feat : support useSuspenseQuery on typescript-react-apollo Sep 10, 2023
@Hal-ang Hal-ang changed the title feat : support useSuspenseQuery on typescript-react-apollo feat[typescript-react-apollo] : support useSuspenseQuery Sep 10, 2023
@Hal-ang Hal-ang changed the title feat[typescript-react-apollo] : support useSuspenseQuery feat : [typescript-react-apollo] support useSuspenseQuery Sep 10, 2023
@Hal-ang
Copy link
Contributor Author

Hal-ang commented Sep 10, 2023

@saihaj
Hi, Could you review this PR? thx :D

@Hal-ang
Copy link
Contributor Author

Hal-ang commented Sep 10, 2023

sorry. If I understand correctly, is it correct that the test failure of github action is resolved when the test code changes in my PR are merged? I'm not sure what part I need to fix separately. This is my first contribution, so please understand my lack of experience. 😭

@saihaj
Copy link
Collaborator

saihaj commented Sep 11, 2023

hey @Hal-ang can you please run yarn prettier . --write and then commit that. That should make the prettier check happy
CleanShot 2023-09-11 at 16 58 04

for the failing tests you need to update all the snapshots I think
CleanShot 2023-09-11 at 16 58 29

@Hal-ang
Copy link
Contributor Author

Hal-ang commented Sep 15, 2023

Thanks! I pushed the new updated snapshot and prettier code

@Hal-ang
Copy link
Contributor Author

Hal-ang commented Sep 23, 2023

HI, @saihaj :D
If you have a time, Could you please merge this PR? Thanks!

@saihaj
Copy link
Collaborator

saihaj commented Sep 25, 2023

@Hal-ang looks like tests are still failing. Are they passing for you locally?

@saihaj
Copy link
Collaborator

saihaj commented Sep 25, 2023

actually I reverted things and CI on main should pass. You should try syncing this branch with main and then that should make CI happy 🤞🏼 @Hal-ang

@saihaj saihaj added the waiting-for-answer Waiting for answer from author label Sep 25, 2023
@Hal-ang
Copy link
Contributor Author

Hal-ang commented Oct 1, 2023

actually I reverted things and CI on main should pass. You should try syncing this branch with main and then that should make CI happy 🤞🏼 @Hal-ang

Oh! Sorry for the late confirmation. thanks I tried to syncing a fork branch and checked test on my local :D
Screenshot 2023-10-01 at 12 04 27 PM

@saihaj
Copy link
Collaborator

saihaj commented Oct 3, 2023

hey @Hal-ang can you run yarn generate:examples and commit the changes?

CleanShot 2023-10-03 at 14 21 51

@Hal-ang Hal-ang force-pushed the feat/useSuspenseQuery branch from e9db367 to 290f32a Compare October 4, 2023 12:24
@Hal-ang
Copy link
Contributor Author

Hal-ang commented Oct 4, 2023

hey @Hal-ang can you run yarn generate:examples and commit the changes?

CleanShot 2023-10-03 at 14 21 51

thank you I tried running that script and running 'yarn run test' from root. After that, I also confirmed that it was passed. However, I am worried because there is an import error in the created folder. 🥲

Screenshot 2023-10-04 at 9 24 55 PM Screenshot 2023-10-04 at 9 26 26 PM

@Hal-ang
Copy link
Contributor Author

Hal-ang commented Oct 9, 2023

It seems that the node-side settings of env in GitHub Actions do not work well. I'm not sure how to solve this, can you help me?
Screenshot 2023-10-09 at 1 16 12 PM

@fforres
Copy link

fforres commented Oct 17, 2023

@saihaj @Hal-ang Really excited to see this feature :D
Anything the rest of us can do to help?
(Would love to use this as a base to add support to useBackgroundQuery 🙏, not sure what the state of the PR is)

@saihaj saihaj removed the waiting-for-answer Waiting for answer from author label Oct 25, 2023
Copy link
Collaborator

@saihaj saihaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work @Hal-ang!

@Brobin
Copy link

Brobin commented Dec 1, 2023

Can we please add to the release notes that this requires @apollo/client > 3.8?

@Grohden
Copy link

Grohden commented Dec 27, 2023

New feature (non-breaking change which adds functionality)

isn't this a breaking change since it now requires apollo >= 3.8 and we can't opt out of suspense generation stuff?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants