Semantic nullability document directive #4271
New GraphQLSemanticNonNull type
Handle isNonNullType
More fixes
More fixes
Yet more updates
Recognize in introspection, enable disabling null bubbling
Lint fixes
More missing pieces
More fixes
Fix schema
Fix another test
More minor test fixes
Fix introspection test
Add support for * to lexer
Allow specifying errorPropagation at top level
Factor into getIntrospectionQuery
Lint
Prettier
parser tests passing
Add semantic optional type
printer and parser tests passing
some new semanticNullability execution tests
SemanticNonNull halts null propagation
SemanticOptional cleared
logging cleanup
rename to SemanticNullable
better SemanticNullable docs
| 1365 | rootValue: data, | ||
| 1366 | }); | ||
| 1367 | |||
| 1368 | let executable = document.definitions?.values().next().value as ExecutableDefinitionNode; | ||
| 1369 | let selectionSet = executable.selectionSet.selections.values().next().value; |
twof227 days agoThese are just tests so it's not the biggest deal, but is there a better way to retrieve the node for use in error construction? This is workable, but unwieldy.
yaacovCR227 days agođź‘€ 1Because we do not yet enable noUncheckedIndexAccess (see #3867) I think you could just index it via 0 at this point although I think what you are doing is preferred, for example
graphql-js/src/execution/subscribe.ts
Line 217 in 423e72d
Has been changed in main to your method.
You could assert that you have an executable definition or selection node instead of using typescript casting….
In fact, I think that needs to be improved in main for the example I gave!
graphql-js/src/execution/execute.ts
Line 2241 in 831c121
yaacovCR227 days agographql-js/src/language/predicates.ts
Line 24 in 423e72d
yaacovCR225 days agoPaid greater attention when working on twof#5 and I think the answer is that generally the tests use expectJSON and some other testing deduplication, added second commit there demonstrating use.
move semantic nullability tests to their own file
fix git status
run prettier
Add comment to parser about 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.
| 740 | 754 | * - NamedType | |
| 741 | 755 | * - ListType | |
| 742 | 756 | * - NonNullType | |
| 757 | * - SemanticNonNullType |
BoD227 days ago| * - SemanticNonNullType | |
| * - SemanticNonNullType | |
| * - SemanticNullableType |
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 twof#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.
Login to write a write a comment.