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

fix(subgraph): Conditionally print @specifiedBy directive on scalars correctly #1463

Merged
merged 6 commits into from
Feb 1, 2022

Conversation

angelo-moreira
Copy link
Contributor

When having a schema with scalars

scalar Date
scalar JSON
scalar HexColour

We end up in a situation where the scalar is undefined instead of null.

Because of that when we try to get the sdl we receive the @specifiedBy tag with an object attached to it.

query getIntrospection {
  _service {
    sdl
  }
}

image

This PR should fix that by weakly comparing undefined == null, I am happy to do a more strict comparison.

@apollo-cla
Copy link

@angelo-moreira: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 1, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@trevor-scheer
Copy link
Member

Thanks @angelo-moreira, this LGTM.

Would you please add a CHANGELOG entry for this in subgraph-js/CHANGELOG.md?

I'll get this cherry-picked / backported to the version-0.x branch.

@trevor-scheer trevor-scheer added the ↪️ backport-to-0.x landed on 2.x, but needs to be backported to the 0.x series label Feb 1, 2022
@angelo-moreira
Copy link
Contributor Author

Thank you for the quick reply @trevor-scheer, I have now added a line to the changelog with hopefully a useful commit, please do let me know if you need me to do something else, it's my first time contributing to this project.

@trevor-scheer trevor-scheer changed the title scalar shouldn't have the @specifiedBy directive fix(subgraph): Conditionally print @specifiedBy directive on scalars correctly Feb 1, 2022
@trevor-scheer
Copy link
Member

A great first contribution @angelo-moreira! I made a few minor tweaks to save you the trouble, but otherwise this is excellent. Thanks again!

@trevor-scheer trevor-scheer merged commit c22fb5c into apollographql:main Feb 1, 2022
trevor-scheer added a commit that referenced this pull request Feb 1, 2022
…correctly (backport #1463) (`version-0.x`) (#1465)

Co-authored-by: Angelo Moreira <angelo.moreira@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
↪️ backport-to-0.x landed on 2.x, but needs to be backported to the 0.x series
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants