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

fixed introspection field "specifiedBy" to "specifiedByURL" #848

Merged
merged 4 commits into from Apr 16, 2021

Conversation

Code-Hex
Copy link
Contributor

@Code-Hex Code-Hex commented Apr 12, 2021

@IvanGoncharov
Copy link
Member

@Code-Hex Thanks for opening it 👍

Context: #649 (comment)
So naming change from the above suggestion was accepted by the introspection field stayed unchanged.
The only argument I saw is:

This is a personal feelings, specifiedByURL causes some associations with the Hungarian notation, which has its drawbacks.

But graphql-js PR added specifiedByURL and I always assumed that #649 is also used that name.
Moreover, other implementations (e.g. Python, graphql-java) followed reference implementation and did specifiedByURL.

Also, I think specifiedByURL is needed to be consistent with deprecatedReason and future directives exposed through introspection.
The original proposal is still Draft - Stage3 so we can fix it before it gets released.

cc: @leebyron

@leebyron leebyron added this to the May2021 milestone Apr 13, 2021
@leebyron leebyron added the 🤷‍♀️ Ambiguity An issue/PR which identifies or fixes spec ambiguity label Apr 13, 2021
@leebyron
Copy link
Collaborator

leebyron commented Apr 13, 2021

Thanks for opening this - I thought we had handled this, but perhaps we missed something.

While you're at it, can you make sure prose is updated accordingly?

Example:

When defining a custom scalar, GraphQL services should provide a specification URL via the @specifiedBy directive or the specifiedBy introspection field.

to

When defining a custom scalar, GraphQL services should provide a specification URL via the @specifiedBy directive or the specifiedByURL introspection field.

@Code-Hex
Copy link
Contributor Author

Code-Hex commented Apr 13, 2021

@leebyron I've done it. Could you review please.

Code-Hex added a commit to Code-Hex/graphql-js that referenced this pull request Apr 14, 2021
@nodkz
Copy link

nodkz commented Jun 2, 2021

I think you made a typo with specifiedByURL, should be specifiedByUrl.

GraphQLJSON https://github.com/taion/graphql-type-json/blob/master/src/index.js#L55-L56 and other scalars distributed via NPM now lost specifiedByUrl property with v16.0.0-alpha.2

PS. The opened issue for this typo: graphql/graphql-js#3156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤷‍♀️ Ambiguity An issue/PR which identifies or fixes spec ambiguity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants