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

Use GraphQL Yoga #4712

Merged
merged 15 commits into from
Mar 21, 2022
Merged

Use GraphQL Yoga #4712

merged 15 commits into from
Mar 21, 2022

Conversation

ardatan
Copy link
Contributor

@ardatan ardatan commented Mar 10, 2022 •
edited
Loading

Hi!
As we discussed recently. We decided to create this PR to migrate your GraphQL Server implementation to GraphQL Yoga.
GraphQL Yoga is platform agnostic GraphQL Server for JavaScript environment that follows Web standards and supports GraphQL-over-HTTP spec together with GraphQL Multipart spec, defer/stream and SSE subscriptions.
https://graphql-yoga.com/
Thanks to Yoga, you don't need any extra configuration and code for Envelop integration, handling and parsing requests, CORS and healthchecks.
Also we saw that you're using node-fetch already for polyfilling Node and abstracting some HTTP stuff. This PR replaces it with cross-undici-fetch which is a smart ponyfill library like node-fetch but chooses the best fetch implementation according to your Node version etc(node-fetch for v14,undici for v16, native fetch for v17 etc).

As far as I see, there is no change in the user facing API (not sure if onHealthCheck is something for users) so in the docs.

We believe you can leverage more from Fetch API like we do in GraphQL Yoga so GraphQL Yoga can have tighter integration. We'd appreciate your feedback. Thanks!

@dthyresson dthyresson added the release:feature This PR introduces a new feature label Mar 10, 2022
@ardatan ardatan marked this pull request as ready for review March 10, 2022 14:17
@ardatan ardatan force-pushed the graphql-yoga branch 3 times, most recently from 2dcb8ef to 27c8499 Compare March 14, 2022 13:00
@dac09
Copy link
Contributor

dac09 commented Mar 15, 2022

@ardatan thank you for this and the crazy quick turn around. The Guild never fails to impress me ♥️

@thedavidprice
Copy link
Contributor

Updated from main to include new CI Auth Smoke Tests via #4673

dthyresson
dthyresson requested changes Mar 17, 2022
View reviewed changes
Copy link
Contributor

@dthyresson dthyresson left a comment

Choose a reason for hiding this comment

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

Had one question about the use of localhost in the hanlderFn.

@netlify
Copy link

netlify bot commented Mar 21, 2022 •
edited
Loading

✅ Deploy Preview for redwoodjs-docs ready!

🔨 Explore the source changes: bf32ca2

🔍 Inspect the deploy log: https://app.netlify.com/sites/redwoodjs-docs/deploys/6238b1d61550bc000822740b

😎 Browse the preview: https://deploy-preview-4712--redwoodjs-docs.netlify.app

@dthyresson
Copy link
Contributor

@dac09 and @ardatan I resolved the conflicts an updated the branch. I still think we have a few questions about use of localhost and perhaps CORS to think about before merge and testing.

@dthyresson
Copy link
Contributor

I just realized I may have added back node-fetch as part of conflict resolution

dthyresson
dthyresson previously approved these changes Mar 21, 2022
View reviewed changes
@dthyresson dthyresson dismissed their stale review March 21, 2022 17:42

Meant to approve the changes not the PR

@dthyresson
Copy link
Contributor

GraphQL Playground running on GitPod

image

@dthyresson dthyresson merged commit ca58e05 into redwoodjs:main Mar 21, 2022
@jtoar jtoar added this to the next-release milestone Mar 21, 2022
@ardatan ardatan deleted the graphql-yoga branch March 21, 2022 18:10
dac09 added a commit to dac09/redwood that referenced this pull request Mar 22, 2022
@dac09
Merge branch 'main' of github.com:redwoodjs/redwood into feat/remove-…
41892ef 
…postprocessing-nested-api-functions

* 'main' of github.com:redwoodjs/redwood:
  Stying updates on docs site
  fix(deps): update typescript-eslint monorepo to v5.16.0 (redwoodjs#4862)
  fix(deps): update dependency cross-undici-fetch to v0.1.27 (redwoodjs#4858)
  fix(deps): update dependency @graphql-yoga/common to v0.1.0-canary-bfd2627.0 (redwoodjs#4857)
  chore(deps): update dependency zx to v6.0.7 (redwoodjs#4861)
  (docs) add Flightcontrol.dev Deploy  (redwoodjs#4826)
  Handle case when requestContext is undefined getting protocol (redwoodjs#4864)
  Check for gql documents before running codegen (redwoodjs#4728)
  change flightcontrol setup to use yarn v3 and change cookie config (redwoodjs#4843)
  Forward -> Foreward
  Forward -> Foreward
  Typo
  Reorgnize tutorial into chapters (redwoodjs#4855)
  Fix linting warnings on new gql function (redwoodjs#4859)
  Use GraphQL Yoga (redwoodjs#4712)
  Fix url for tutorial path (redwoodjs#4852)
  chore(deps): update actions/cache action to v3 (redwoodjs#4847)
  Update link to tutorial deployment
  try learn's algolia config (redwoodjs#4851)
@thedavidprice thedavidprice modified the milestones: next-release, v0.50.0 Mar 23, 2022
@borekb
Copy link

borekb commented Apr 2, 2022

Just to confirm, this entirely replaces Helix, right? Helix is still mentioned in places like README and a few others but that might just be something that yet needs updating.

BTW were there any issues with Helix or is Yoga just easier / more convenient to use? We'll be doing a similar decision soon 😊.

@dthyresson
Copy link
Contributor

Hi @borekb

Just to confirm, this entirely replaces Helix, right?

Correct, the RedwoodJS GraphQL Server uses Yoga 2.0 now instead of Helix.

Helix is still mentioned in places like README and a few others but that might just be something that yet needs updating.

Thanks! We do need to clean up those mentions. Appreciate you finding them.

BTW were there any issues with Helix or is Yoga just easier / more convenient to use?

No, no issues with Helix. RedwoodJS decided to use Yoga based on the collaboration with the Guild.

While @dotansimha can explain more, in short, (my take) Yoga takes the "best parts" of Helix, adds in what the Guild had learned from their customers using it in production, better integrates their envelop plugin system, and is now a project that they can maintain, support and improve faster and more easily than Helix (which is maintained by another party).

We'll be doing a similar decision soon

Sounds great. The transition for Redwood was really smooth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thedavidprice thedavidprice thedavidprice left review comments

@dac09 dac09 dac09 approved these changes

@dthyresson dthyresson dthyresson left review comments

Assignees
No one assigned
Labels
release:feature This PR introduces a new feature
Projects
No open projects
Release
Status: Archived
Milestone
v0.50.0
Development

Successfully merging this pull request may close these issues.
6 participants
@ardatan @dac09 @thedavidprice @dthyresson @borekb @jtoar