[spawnia]

Part of #149

Servers omit fields under @skip or @include, but the generated classes require them.
This generates those fields nullable on the current code generation and lets them be missing.

Alternatives: #148 (full support on the merged field model), #146 (fields only), #143 (reject the directives)

A field under `@skip` or `@include` becomes nullable and may be missing from the response

• This applies to the directive on the field itself, on an enclosing inline fragment or on a fragment spread.
• The field gets a nullable @property and an optional make() parameter.
• OmittableConverter marks such fields, and ObjectLike::fromGraphQL() tolerates a missing field only behind it.
  The NonNullConverter stays inside, so an explicit null for a non-null field still fails.
• @skip(if: false) and @include(if: true) count as unconditional.
• A field is required as soon as one of its selections is unconditional, __typename always.
• Sailor does not evaluate the directive arguments at runtime, so it also accepts a missing field the directive included.

Fields under a conditional selection may be missing only when another selection still provides their parent

singleObject @skip(if: $v) { value } singleObject { nested { value } } makes value omittable.
singleObject @include(if: $v) { value } alone keeps value required.

A second pass over the document counts selections per response path and possible parent object type.
OperationGenerator::isOmittable() checks those counts for the ancestors of each field.

The counting is an approximation: two selections under the same condition make their fields omittable, although both are present together.
#148 derives this exactly from a merged field model.

Generated classes change for operations that use the directives

Non-null fields under the directives become nullable.
Their make() parameters become optional and move behind the required ones, so positional make() calls need updating.

`ObjectLikeBuilder` signatures change

addProperty() requires a new bool $isOmittable argument.
buildProperty() takes a PropertyDefinition instead of positional arguments.

🤖 Generated with Claude Code

[CRC-Mismatch]

I was checking #77, #78 and this for an upcoming task I'm about to take on later this week or the next.

Is it currently already possible to properly use the directives, or are there still any limitations that this PR is meant to solve?

TL;DR - We need to suppress unneeded queries and results to reduce the performance penalty

We're transitioning back-ends from an close-to-unmaintainable REST-only system to a new back-end designed from scratch with GraphQL-only support in mind. To enable the front-end developers to "chop" the transitioning so we can deliver a production-ready migration without having to delay everything for months (or years), we've developed a BFF that "translates" the most coupled legacy REST contracts and endpoints to the new model using Sailor. I've developed a Symfony bundle wrapper (mismatch/sailor-bundle) to ease its integration in our current projects' architecture.

Problem is, we can't deliver the BFF (and by association the clients' migration) as "production-ready" until we can solve the huge performance penalty due to making one REST request become ~15 GraphQL queries and mutations (most of the queries are already compressed in 3 requests, but the mutations are mostly one-per-request) since when we originally started, we couldn't leverage the @include and @skip directives.

Without them, we'll have to create one variation of each query or mutation for each possible scenario where we don't need something, which could solve the performance problem with some clever coding to decide when to use what; but that would mean having around 4x the number of generated classes, and the same amount of new code-paths (incurring in huge NPath and Cyclomatic complexities, in a code that already suffers from these).

I'd like to know the current status, as when I take on that task, if something is still amiss, I'm probably going to first try and work on this PR, instead of turning our BFF's code into a Hydra right away.

[spawnia]

Is it currently already possible to properly use the directives, or are there still any limitations that this PR is meant to solve?

No, the problems with them are detailed in the description of this pull request. Sailor will currently not handle a field missing due to @skip or @include gracefully. Feel free to fork and propose a solution to this pull request, it is currently only a failing test.

[Copilot]

Pull request overview

This PR adds support for GraphQL's @skip and @include client directives, allowing fields to be conditionally omitted from queries. When these directives are used, the implementation marks affected fields as nullable in generated classes and gracefully handles their absence in server responses, addressing issue #77.

Changes:

• Modified code generation to unwrap non-null types for fields with @skip or @include directives
• Updated ObjectLike deserialization to skip missing fields when they use NullConverter
• Added comprehensive tests for client directives on fields, fragment spreads, and inline fragments

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/Codegen/OperationGenerator.php	Added logic to detect @skip/@include directives on fields and unwrap NonNull types to allow nullability
src/ObjectLike.php	Modified field deserialization to skip missing fields when using NullConverter instead of throwing exceptions
examples/simple/src/clientDirectives.graphql	Added test queries for SkipNonNullable and IncludeNonNullable operations
examples/simple/schema.graphql	Added nonNullable: String! field to Query type for testing
examples/simple/expected/Operations/SkipNonNullable/*.php	Generated classes for SkipNonNullable operation showing NullConverter usage
examples/simple/expected/Operations/IncludeNonNullable/*.php	Generated classes for IncludeNonNullable operation showing NullConverter usage
tests/Integration/SimpleTest.php	Added 12 new test methods covering various scenarios of fields being omitted or present with client directives
README.md	Added documentation section explaining client directive support with code examples
CHANGELOG.md	Added entry for v1.3.0 documenting this fix

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

@spawnia I've opened a new pull request, #136, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Copilot review overview

🟡 Changes recommended

Response-path counting conflates mutually exclusive concrete-type branches, producing incorrectly omittable fields and weakened response validation.

Review effort: Balanced
Findings: 1

Open (1)

• Runtime type branches incorrectly mark required fields as omittable · New

[spawnia]

🤖 Superseded by #148, which builds on the merged field model of #144.