graphql
Semantic nullability document directive
Notes for reviewers
I'm looking for review on everything but utilities. I'm going to let @benjie handle those. Once I've been given a go ahead on the implementation, I'll write the spec edits.
There are a large number of files failing linting. Let me know if there's anything I need to do to fix that.
Coverage is passing on my machine. Unclear why it's not passing CI. It also seems to be treating one of my new test files as uncovered code.
Is there a reason for the introduction of two new wrapping types as opposed to a single one? Is it because of an unresolved syntax question or is it expected to remain in the final implementation?
I could see this being useful if we were to actually allow multiple coexisting syntax variants as dictated by arguments on the semantic nullability directive (but I am not in favor of that additional complexity).
Is there a reason for the introduction of two new wrapping types as opposed to a single one? Is it because of an unresolved syntax question or is it expected to remain in the final implementation?
I could see this being useful if we were to actually allow multiple coexisting syntax variants as dictated by arguments on the semantic nullability directive (but I am not in favor of that additional complexity).
In the first commit with the parsing implementation I had a single type.
The printer was the main driver here. In the parser there was a clear place to set the useSemanticNullability mode flag, but with the printer it wasn't so obvious.
The printer was the main driver here. In the parser there was a clear place to set the
useSemanticNullabilitymode flag, but with the printer it wasn't so obvious.
I raised https://github.com/twof/graphql-js/pull/5 as a proof-of-concept for what it might look like with a single new wrapper type.
I stored within the created GraphQLSchema whether it was created usingSemanticNullability and built printing and execution related changed off of that flag.
You can add in semantically nullable types even in a schema that is not using usingSemanticNullability and they will be ignored. In particular, to remain with simply one version of the Introspection types, I modified the built-in Introspection types where I thought to be appropriate to use the new GraphQLSemanticNullable wrapping types; these types are unwrapped but otherwise ignored even when usingSemanticNullability is not set.