Skip to content

fix: Undefined input handling (refuse explicit null, default to Undefined) and list-formatted description metadata - #838

Merged
oojacoboo merged 2 commits into
thecodingmachine:masterfrom
oojacoboo:fix/undefined-refuses-explicit-null
Oct 10, 2026
Merged

oojacoboo merged 2 commits into
thecodingmachine:masterfrom
oojacoboo:fix/undefined-refuses-explicit-null

Conversation

@oojacoboo

Copy link
Copy Markdown
Collaborator

What

Four related changes to how GraphQLite handles Undefined input, plus one fix to the docblock description toggle from #793:

  1. An explicit null for an argument or input field typed T|Undefined (without null) is refused with a client error instead of a TypeError.
  2. Undefined takes precedence over the implicit null default, so an omitted T|null|Undefined field resolves to Undefined::VALUE.
  3. Undefined-typed arguments and input fields say in their description whether null is accepted.
  4. Description metadata (the #[Cost] line, the sentence above, and anything a middleware adds) is rendered as a Markdown list after the description.
  5. Argument @param descriptions respect setDocblockDescriptionsEnabled(false).

1. Refusing an explicit null for T|Undefined

GraphQL can't express "may be omitted, but not null": an input field is optional only if it is nullable (Int), or if it has a default that hides the omission (Int! = 5). So UndefinedTypeMapper exposes int|Undefined as Int, which is correct, but when a client then sends null, GraphQLite assigns it to a property (or passes it to a setter, constructor parameter or method argument) whose PHP type can't hold it, and the request fails with a TypeError.

Now, when the PHP type that receives the value contains Undefined but not null, an explicit null is refused before anything is assigned or the resolver runs:

Argument 'age' may be omitted but cannot be null

Parameters\NullArgumentException sits next to MissingArgumentException and extends GraphQLException, so it is client-safe. The check runs in InputTypeParameter::resolve(), which every hydration path goes through: public properties, setters, __call, promoted and plain constructor parameters, #[Factory] parameters, query/mutation arguments, lists, and variables bound to null. Nullability is decided by the sink that actually receives the value (a constructor parameter or setter can differ from the property it fills). T|null|Undefined still accepts null. This follows the spec editor's guidance in graphql/graphql-spec#476 to enforce "optional non-null" with server-side validation.

2. Undefined before the implicit null default

TypeHandler::mapInputProperty applied the nullable rule (default = null) before the Undefined rule, while mapParameter checked them in the opposite order. So int|null|Undefined $x; without a PHP default printed = null, webonyx filled null when the field was omitted, and it never resolved to Undefined. The docs example protected int|null|Undefined $age; hit this.

Now Undefined is checked first in both. An omitted field without a PHP default is hydrated with Undefined::VALUE (via InputField::resolveOmitted()), including properties left uninitialized and required constructor parameters, unless the constructor already assigned the property. Phpdoc-only types (@var int|null|Undefined) are handled too. A typed property with an explicit default (= null, = 5) keeps it and still prints it.

3. Saying whether null is accepted

Applied directives aren't exposed by introspection (graphql/graphql-spec#300) and webonyx's SchemaPrinter doesn't print custom ones, so the description is the only place every client sees the rule. Undefined-typed arguments and input fields get one of:

  • May be omitted; null is not accepted. (T|Undefined)
  • May be omitted; null is accepted. (T|null|Undefined)

It isn't added when the exposed GraphQL type is non-null (for example forced with inputType: 'Int!'), and it is never duplicated. SchemaFactory::setUndefinedDescriptionsEnabled(false) turns it off, matching setDocblockDescriptionsEnabled(). It is on by default: a description change doesn't break clients, only schemas that use Undefined are affected, and those fields' printed defaults change in this release anyway.

4. Description metadata as a list

QueryFieldDescriptor and InputFieldDescriptor keep the description separate from appended metadata items, and DescriptionResolver::appendItems() renders them once:

"""
Paginated list of books, optionally filtered

- Audience: librarians
- Cost: complexity = 5, multipliers = [first], defaultMultiplier = null
"""
books(first: Int): [Book!]!

Middlewares add items with the new withAddedDescriptionItem(). Items keep middleware order, survive withDescription(), and are de-duplicated (including against text the description already contains). QueryFieldDescriptor::withAddedDescriptionLines() keeps its behaviour and is deprecated with trigger_error, as other deprecations in the codebase are.

5. #793 follow-up

TypeHandler::mapParameter computed the @param description behind the docblock toggle and then overwrote it unconditionally, so argument descriptions still appeared with setDocblockDescriptionsEnabled(false). The overwrite is removed.

Breaking changes

Documented in the 8.4.0 CHANGELOG with migration notes:

  • The #[Cost] text in introspection and printed schemas becomes the list item - Cost: complexity = .... Anything parsing or snapshotting descriptions needs updating.
  • An omitted T|null|Undefined field without a PHP default resolves to Undefined::VALUE instead of null and no longer prints = null. Code treating null as "not sent" should compare against Undefined::VALUE.

Tests

  • End-to-end coverage for every hydration path above, including input classes with no constructor (public properties, and private properties hydrated through __call), with and without PHP defaults: omitted gives Undefined, a value comes through, null is accepted or refused as typed, refusals run under DebugFlag::RETHROW_UNSAFE_EXCEPTIONS so a TypeError fails the test, and variables bound to null or left out.
  • Description tests for the sentence on every path, with and without a developer description, the toggle off, forced non-null types, the list rendering, ordering, cached rebuilds, and the feat: explicit description attribute (+ #[EnumValue]) + SchemaFactory docblock toggle #793 toggle on arguments.
  • Each fix was confirmed by reverting it locally and watching the new tests fail.
  • Full suite: 724 tests. CI passed on PHP 8.2 to 8.5, with and without --prefer-lowest, on the fork.

…ined) and list-formatted description metadata

- An explicit null for an argument or input field whose receiving PHP type contains Undefined but not
  null is refused with NullArgumentException, naming the field, instead of failing with a TypeError
- Undefined takes precedence over the implicit null default, so an omitted T|null|Undefined field
  without a PHP default resolves to Undefined::VALUE and prints no `= null`; omitted fields without a
  default are hydrated with Undefined unless the constructor assigned them
- Undefined-typed arguments and input fields say in their description whether null is accepted,
  controlled by SchemaFactory::setUndefinedDescriptionsEnabled()
- Description metadata (Cost, the Undefined sentence, middleware items) renders as a Markdown list via
  withAddedDescriptionItem(); withAddedDescriptionLines() is deprecated
- Argument @PARAM descriptions respect setDocblockDescriptionsEnabled(false) (thecodingmachine#793)
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.72727% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.08%. Comparing base (53f9d49) to head (fec72e3).
⚠️ Report is 178 commits behind head on master.

Files with missing lines Patch % Lines
src/InputFieldDescriptor.php 75.00% 2 Missing ⚠️
src/FieldsBuilder.php 97.56% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #838      +/-   ##
============================================
- Coverage     95.72%   92.08%   -3.65%     
- Complexity     1773     2158     +385     
============================================
  Files           154      204      +50     
  Lines          4586     5746    +1160     
============================================
+ Hits           4390     5291     +901     
- Misses          196      455     +259     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@oojacoboo
oojacoboo merged commit 5310e18 into thecodingmachine:master Oct 10, 2026
12 checks passed
@oojacoboo
oojacoboo deleted the fix/undefined-refuses-explicit-null branch October 10, 2026 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants