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

Support returning async iterables from resolver functions #2757

Merged
merged 2 commits into from
Jun 23, 2022
Merged

Support returning async iterables from resolver functions #2757

merged 2 commits into from
Jun 23, 2022

Conversation

IvanGoncharov
Copy link
Member

@IvanGoncharov IvanGoncharov commented Aug 25, 2020 •
edited by robrichard
Loading

Continuation of #2712

Depends on #3134

@IvanGoncharov IvanGoncharov added the PR: feature 🚀 requires increase of "minor" version number label Aug 25, 2020
@IvanGoncharov
Copy link
Member Author

@robrichard Since ATM we are using a very similar algorithm I'm worried that we will also have memory leak:
leebyron/iterall#24
So working on adding performance tests against it.

@IvanGoncharov IvanGoncharov force-pushed the asynciteratable branch 2 times, most recently from f612eb6 to d25cf40 Compare October 13, 2020 23:14
@gagoar gagoar mentioned this pull request Nov 27, 2020
Closed
Base automatically changed from master to main January 27, 2021 11:10
@robrichard robrichard mentioned this pull request Apr 6, 2021
Closed
@robrichard robrichard mentioned this pull request May 26, 2021
Merged
@robrichard robrichard force-pushed the asynciteratable branch 2 times, most recently from f1ddb83 to 10f12a1 Compare June 2, 2021 00:45
@robrichard robrichard mentioned this pull request Jun 2, 2021
Closed
@robrichard robrichard force-pushed the asynciteratable branch 4 times, most recently from e93482c to b393729 Compare June 4, 2021 15:10
@robrichard robrichard force-pushed the asynciteratable branch 2 times, most recently from 6415aa1 to 93bd84a Compare August 30, 2021 16:22
@yaacovCR yaacovCR mentioned this pull request Oct 25, 2021
Merged
@github-actions github-actions bot mentioned this pull request Oct 26, 2021
Merged
@robrichard robrichard force-pushed the asynciteratable branch 2 times, most recently from 2153dda to fa38819 Compare October 28, 2021 20:56
@robrichard robrichard force-pushed the asynciteratable branch 3 times, most recently from c2d20a1 to 971ce43 Compare November 19, 2021 15:09
@robrichard robrichard force-pushed the asynciteratable branch 4 times, most recently from e8c1a70 to 09d56d6 Compare December 1, 2021 19:50
@github-actions
Copy link

The latest changes of this PR are available on NPM as
graphql@16.3.0-canary.pr.2757.7e793c133f5b9c47a1b12e98ea3a35b1b1471dc4
Note: no gurantees provided so please use your own discretion.

Also you can depend on latest version built from this PR:
npm install --save graphql@canary-pr-2757

@netlify
Copy link

netlify bot commented Apr 4, 2022 •
edited
Loading

✅ Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
🔨 Latest commit 2826e53
🔍 Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/62b34f3315cfec0008fd799f
😎 Deploy Preview https://deploy-preview-2757--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@saihaj saihaj requested review from yaacovCR and robrichard April 23, 2022 04:37
yaacovCR
yaacovCR requested changes Apr 24, 2022
View reviewed changes
Copy link
Contributor

@yaacovCR yaacovCR left a comment

Choose a reason for hiding this comment

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

My review (requested by @saihaj ) is basically that we should allow resolvers to return async iterables of iterables, not of individual items. See graphql/defer-stream-wg#38

@robrichard robrichard force-pushed the asynciteratable branch 2 times, most recently from b742343 to 92ef2af Compare May 5, 2022 14:58
@robrichard robrichard force-pushed the asynciteratable branch 3 times, most recently from ab235c2 to 18a6145 Compare May 26, 2022 12:57
@IvanGoncharov IvanGoncharov requested a review from a team May 26, 2022 20:46
yaacovCR
yaacovCR approved these changes May 29, 2022
View reviewed changes
src/execution/execute.ts Show resolved Hide resolved
@yaacovCR
Copy link
Contributor

We decided at last js-wg to give this a few days, but it seems to have slipped. It looks good to me. @IvanGoncharov @saihaj ?

@IvanGoncharov IvanGoncharov merged commit 59c87c3 into main Jun 23, 2022
@IvanGoncharov IvanGoncharov deleted the asynciteratable branch June 23, 2022 18:19
@IvanGoncharov
Copy link
Member Author

@robrichard Merged, sorry for the long overdue review.
Hope it will help to accelerate steam/defer RFC 🚀

angelini pushed a commit to gadget-inc/graphql-js that referenced this pull request Aug 30, 2023
@IvanGoncharov @robrichard @angelini
Support returning async iterables from resolver functions (graphql#2757)
85c257f 
Co-authored-by: Rob Richard <rob@1stdibs.com>
@jasonkuhrt jasonkuhrt mentioned this pull request Oct 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dnalborczyk dnalborczyk dnalborczyk left review comments

@saihaj saihaj saihaj approved these changes

@yaacovCR yaacovCR yaacovCR approved these changes

@robrichard robrichard Awaiting requested review from robrichard

Assignees
No one assigned
Labels
PR: feature 🚀 requires increase of "minor" version number
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@IvanGoncharov @yaacovCR @dnalborczyk @robrichard @saihaj