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

chore(deps): Update graphql and related dependencies #1200

Merged
merged 3 commits into from Nov 19, 2021

Conversation

trevor-scheer
Copy link
Member

No description provided.

@trevor-scheer
Copy link
Member Author

Is there any reason we'd want to continue supporting graphql@14.x or really anything besides more cutting edge versions for Fed 2? Can we get to 16+ before launch?

I'd like to get gateway supporting v16 asap, this seems like a good first step. Unfortunately there's a minor version bump in graphql-js which breaks @apollo/core-schema@0.1.0. The fix requires getting peer deps up to ^15.7.x.
Ref: apollographql/core-schema-js#20

Updates on the TypeScript query planner changed the PlanningError structure, that will now always have an `extensions` field.
No extensions are now expressed by having the extensions field value being null, or an empty Object.

This PR allows us to express these requirements by:
- Adding a new deserialize directive `none_only_if_value_is_null_or_empty_object`.
- Applying the directive to the `PlanningError::extensions` field.
- Adding comments and tests that make the new requirement more explicit.
@trevor-scheer trevor-scheer self-assigned this Nov 12, 2021
@clenfest
Copy link
Contributor

I'm not qualified to judge the Rust stuff, but if it compiles, it's probably fine, right? Otherwise LGTM.

@trevor-scheer
Copy link
Member Author

@clenfest fortunately the Rust stuff is just cherry-picked from the version-0.x branch PR which addresses this same issue #1149

Copy link
Contributor

@clenfest clenfest left a comment

Choose a reason for hiding this comment

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

Approving since Rust changes are approved already in #1149

@trevor-scheer
Copy link
Member Author

Leaving this for @pcmanus or @martijnwalraven to merge

Copy link
Contributor

@pcmanus pcmanus left a comment

Choose a reason for hiding this comment

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

Lgtm as well.

@pcmanus pcmanus merged commit 8e28673 into main Nov 19, 2021
@pcmanus pcmanus deleted the trevor/update-v2-deps branch November 19, 2021 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants