Skip to content

Validation Too Strict for Custom Scalars #1705

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

Closed
gaberogan opened this issue Oct 31, 2020 · 7 comments
Closed

Validation Too Strict for Custom Scalars #1705

gaberogan opened this issue Oct 31, 2020 · 7 comments

Comments

@gaberogan
Copy link

Custom Scalars is GraphQL's only way of providing flexible input types. As it should be, there is no validation for these in the main editor. In the variable editor however, a custom scalar is only allow to be a string, number, boolean, or null. Anything else throws an error. If I need a free form object in the variable editor, it will complain. Could we make a non-breaking change and remove validation for custom scalars here to be more consistent and flexible, allowing free form object types? (by removing type instanceof GraphQLScalarType)

// Validate enums and custom scalars.
if (type instanceof GraphQLEnumType || type instanceof GraphQLScalarType) {
if (
(valueAST.kind !== 'String' &&
valueAST.kind !== 'Number' &&
valueAST.kind !== 'Boolean' &&
valueAST.kind !== 'Null') ||
isNullish(type.parseValue(valueAST.value))
) {
return [[valueAST, `Expected value of type "${type}".`]];

Screenshot 2020-10-31 193830

@acao
Copy link
Member

acao commented Oct 31, 2020

hey, good question!

maybe instead it could provide a warning, such as how vscode gives a warning squiggle for any or implicit any even when "noImplicitAny": "true"

also of interest, there is a 2nd stage draft spec for Custom Scalar Definitions that are hosted via URIs:
graphql/graphql-spec#635

there is also this successor to the inputUnion spec proposal that is now RFC 1, called tagged type:
graphql/graphql-spec#733

@gaberogan
Copy link
Author

@acao Thanks for the input, I haven't seen those RFCs before. I think in the case of a custom scalar it would be an explicit "any" because the variable editor knows it's a custom scalar and unfortunately we can't perform any validation beyond this and have to assume it's valid.

@acao
Copy link
Member

acao commented Nov 4, 2020

personally, i feel it's best to make it invalid by default until we adopt the custom scalars spec. otherwise it's a breaking change. i prefer it to be invalid by default, because JSON, etc custom scalars, while sometimes useful, are less than ideal (like any in typescript). the custom scalars spec will take care of the rest, and then GraphiQL will know how to properly validate most any custom scalar in use

@acao
Copy link
Member

acao commented Nov 4, 2020

do you mind if i turn this into a feature request to support the custom scalar spec?

otherwise, it makes the most sense to mark this as a wontfix and close it.

@truptivishe-gep
Copy link

I too am facing this issue as one of my field is of type AnyScalarGraphType. Graphiql shows the warning : Expected value of type "_Any". Shouldn't AnyScalarGraphType be included as a valid scalar type?

@acao
Copy link
Member

acao commented Sep 10, 2021

@truptivishe-gep Any is not a specification scalar, and if you follow the discussion here, you'll see that there is still something in the works for providing and validating custom scalar definitions across languages

@acao
Copy link
Member

acao commented Sep 10, 2021

closing this as a dupe because it's another request for validating the Any scalar/non-spec scalars

graphiql is a to-spec tool used for specification development, and closely aligned with graphql-js, so once the custom scalar spec lands in graphql@17 hopefully we will create a feature request to add this capability.

@acao acao closed this as completed Sep 10, 2021
@acao acao added the wontfix label Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants