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

Rebase of "Schema scalar of type" #1173

Closed

Conversation

IvanGoncharov
Copy link
Member

As we discuss during WG I'm very interested in this feature and willing to be champion for it.
As the first step, I rebased #914

This proposes an additive change which allows custom scalars to be defined in terms of the built-in scalars. The motivation is for client-side code generators to understand how to map between the GraphQL type system and a native type system. As an example, a `URL` custom type may be defined in terms of the built-in scalar `String`. It could define additional serialization and parsing logic, however client tools can know to treat `URL` values as `String`. Presently, we do this by defining these mappings manually on the client, which is difficult to scale, or by giving up and making no assumptions of how the custom types serialize.

Another real use case of giving client tools this information is GraphiQL: this change will allow GraphiQL to show more useful errors when a literal of an incorrect kind is provided to a custom scalar. Currently GraphiQL simply accepts all values.

To accomplish this, this proposes adding the following:

* A new property when defining `GraphQLScalarType` (`ofType`) which asserts that only built-in scalar types are provided.

* A second type coercion to guarantee to a client that the serialized values match the `ofType`.

* Delegating the `parseLiteral` and `parseValue` functions to those in `ofType` (this enables downstream validation / GraphiQL features)

* Exposing `ofType` in the introspection system, and consuming that introspection in `buildClientSchema`.

* Adding optional syntax to the SDL, and consuming that in `buildASTSchema` and `extendSchema` as well as in `schemaPrinter`.

* Adding a case to `findBreakingChanges` which looks for a scalar's ofType changing.
@leebyron
Copy link
Contributor

Thanks for doing this! If you don't mind, I'm going to merge this back into the original PR just to keep tracking of the issue consistent.

I'm excited you're interested in this and are helping to champion it! I think now that the SDL is close to complete we can address this next

leebyron added a commit that referenced this pull request Dec 20, 2017
@IvanGoncharov
Copy link
Member Author

If you don't mind, I'm going to merge this back into the original PR just to keep tracking of the issue consistent.

@leebyron This was my original intention 👍

I'm excited you're interested in this and are helping to champion it! I think now that the SDL is close to complete we can address this next

Meanwhile, I can work on adding a few more tests as a separate PR against schema-scalar-of-type branch. Anything else I can do to help with this PR?

@leebyron
Copy link
Contributor

I just updated that PR, so I'll close this - feel free to submit PRs against that branch with improvements and new tests.

I think tests will be the most helpful, that will help highlight any remaining issues.

IvanGoncharov pushed a commit to IvanGoncharov/graphql-js that referenced this pull request Dec 21, 2017
@IvanGoncharov IvanGoncharov deleted the schema-scalar-of-type-rebase branch January 10, 2018 21:38
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.

3 participants