Semantic nullability rfc implementation

[JoviDeCroock]

Introduction

This is the working implementation for semantic nullability, this originates from work done by @twof and @benjie in #4271 and #4192. This PR intends to supersede this work and drive it home.

When Semantic nullability is on we change the semantics of the schema-type system, currently there are two types of nullability, it's either nullable or not. When semantic nullability is introduced we expand this, any field can be null, non-null or semantically null.
Semantically null fields can only return a null value when there's an error associated with the field, when we return null and there's no error associated we'll bubble up the error to the nearest concretely nullable field.

Semantic nullability will be introduced either by means of a directive in the SDL @SemanticNullability which converts the SDL document to use semantic nullability or by switching on the parse configuration.

Documentation

One of the most important parts for adoption would be to adequately document this feature and rope in folks maintaining schema builder libraries.

Open questions

Currently we employ quite a bit of new semantics to achieve the scenario, we introduce both a semantic null and non-null type. This might confuse some people in terms of explicitness but helps a lot with the print implementation as it clearly delineates the new behaviour types.

My proposal would be to only introduce 1 new AST type which would be SEMANTIC_NON_NULLABLE, the complication would arise for the printer but it would be the least changes for folks creating schema builders and other tools in our ecosystem.

Should we be more concrete about input-types not being able to express semantic nullability?

There is a lot of work still to be done on test coverage but I wanted to raise the PR already to let y'all know that I'm working on it. This is a place for discussion as well, any alternatives can be raised, I am still familiarising myself more on the matter and will probably be doing so for a few days.

Update (5th of February 2025)

We merged 869ca46 which reduces the increase in AST-nodes to 1, this helps us a bit in terms of introducing new concepts and backwards compatability of AST traversal. In an ideal world we would only introduce a new syntax so this becomes easier for printing as well and is backwards compatible when it comes to SDL.

[github-actions]

Hi @JoviDeCroock, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

• @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
• @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

[yaacovCR]

Amazing you are taking this up. I posted #4338 which is just the latest hacking I did on the work done by @benjie and @twof, tests seem to pass there with just the one wrapping type.

[yaacovCR]

Don't have the bandwidth to progress there further, just posting in case that helps.

[JoviDeCroock]

@yaacovCR That does seem to be the inverse of what I'm proposing, I would only have a wrapping type for semantic-nullability which in your PR seems to make the newly introduced type the nullable one, I think that for clarity of the community we would do good in making the SEMANTIC_NON_NULL the only newly introduced AST node. I have yet to explore the feasibility though, I got caught up in a few things

[yaacovCR]

@JoviDeCroock I find this feature very difficult in terms of the upgrade story => definitely part of the challenge of making this work. I see the advantage in having the new wrapper type be the new SemanticNonNull wrapping type, but I do see a parallel here in the debate within the SDL in terms of whether to introduce a new notation such as Type* to indicate SemanticNonNull versus Type? to indicate SemanticNull.

My pre-existing PR, now rebased in #4338, like this PR and like #4271 (as opposed to #4192) is adopting Type? for the SDL, so I assumed that for consistency, we would do the same in terms of AST nodes. As you demonstrate in #4339, that is not strictly necessary, we could come to different answers in terms of the SDL and the AST nodes in terms of what the unadorned type means, but that -- to me -- adds an additional layer of complexity to this already complex story.

Either way, I think the upgrade story is a bit confusing, however. For example, we have a function called getNullableType() which takes a possible type possible wrapped by a GraphQLNonNull and returns the unadorned type. With the approach in this PR, it can still return an unadorned type. With my approach in #4338, this returns a type that is nullable in the old sense, but in the new sense, it is unadorned, so it is not semantically nullable, => confusing!

My feedback to the spec folks if they are looking for it is that I find it extremely difficult to reason constantly in terms of the two different types of nullability and that we should separate these concepts more broadly into different terms, for example, perhaps Catching fields and Optional fields, where Catching fields can be null for an error, and Optional fields are semantically nullable. And then if we wanted to have the base unadorned type be catching and non-optional, our two wrapping types would be something like GraphQLNonCatching and GraphQLOptional.

[benjie]

I'd advise against using "optional" on output types, it implies that the field itself can be omitted, rather than that it can be null (i.e. the difference between type T = {foo?: number} and type T = {foo: number | null} in TypeScript if you pretend that undefined and null are the same value 😉 )

[twof]

when we return null and there's no error associated we'll bubble up the error to the nearest concretely nullable field.

I think we corrected this in Discord, and it looks like SemanticNonNullable has the correct behavior in code.

[twof]

Outstanding TODO. We might want to say something here about how this is being used and include a link to the RFC or spec edits.

[twof]

I think anything printed with allowSemanticNullability, will need to be prefixed with the document directive (@SemanticNullability) because otherwise the contents are liable to misinterpretation.

[benjie]

It's less allow and more just on or off, right? What it's set to changes how a type is interpreted:

	Traditional	@semanticNullability
Int?	ERROR!	Int
Int	Int	SemanticNonNull
Int!	NonNull	NonNull

Document directives must come at the top of the document, and in my mental model they effectively change the "mode" of the rest of the parsing. You might think of other document directives that might change the mode, e.g. @commentsAreDescriptions to re-enable the ancient parsing feature where comments (rather than strings) before fields would become descriptions; or @prefixedDirectives which might put directives before things rather than after. (I'm not proposing either of these be made real, I'm just trying to show that the space for document directives is not just a set of size 1.)

Once you hit your first non-ignored non-document-directive token the mode is then frozen, and you can pass that mode into parseType and other parsing related functions.

Thus I think it would be more like:

Suggested change

	it('prints SemanticNullableType', () => {
	expect(
	print(parseType('MyType?', { allowSemanticNullability: true }), {
	useSemanticNullability: true,
	}),
	).to.equal(dedent`MyType?`);
	});
	it('prints SemanticNullableType', () => {
	expect(
	print(parseType('MyType?', { mode: { semanticNullability: true }}), {
	useSemanticNullability: true,
	}),
	).to.equal(dedent`MyType?`);
	});

I'm not sure "mode" is really the right name to use for the result of applying document level directives, suggestions welcome. It could be that we just take the document-level directives and store their literal values into the object; for example:

@semanticNullability
@behavior(onError: PROPAGATE, onDeprecated: ERROR)
@compositeScalars(serialize: true)

{ __typename }

might have a "mode" of:

const parseOptions = {
  mode: {
    semanticNullability: {},
    behavior: { onError: "PROPAGATE", onDeprecated: "ERROR" },
    compositeScalars: { serialize: true },
  }
}

[benjie]

(Note that this mode should also be an output of parsing, e.g. we might use a document directive in an executable document to enable @noErrorPropagation - and we'll need to see that. Originally I thought that @noErrorPropagation would go on the operation itself rather than the document, but the issue with that is that fragments will have two modes if they're used in two operations with different @noErrorPropagation settings, so codegen becomes complicated. Instead, making it a document directive means it applies to all operations and fragments in a single document in unison.)

[twof]

Great work! Thanks for putting on the finishing touches. I've left some comments, and it looks like we're missing some code coverage, but after that I think we'll be good to go.

[twof]

Oh one thing I missed. I think we need to define the document directive in directives.ts. The handling of the directive might also be better. I did it in a super adhoc way where I'm just looking for the string during parsing.

[JoviDeCroock]

CC @twof @benjie how do we expect to tackle this, document directives aren't allowed as it stands and we'd need to store this on the schema itself. We do have the possibility of a schema directive but that might span different documents.

Similar thoughts arise for String(GraphQLOutputType) and inspect(type) when do we print ? it needs to have some brand on every type that it belongs to a semantic non-null document.