feat: support @include/@skip directives and a pluggable generation lifecycle#68
Merged
Conversation
…ifecycle Conditionally-selected response fields (via @include/@Skip, or fields under a conditional inline fragment / fragment spread) are now generated as nullable, since the server may omit them even when the schema marks them non-null. - Add a generation lifecycle (Grephql.Generation.Plugin) with named steps normalize -> resolve -> lower -> create and after_<step> hooks. @include and @Skip ship as the built-in SkipInclude plugin; users register custom plugins via the :generation_plugins option on `use Grephql`. - Validate variables used in directive arguments. - Cover directive parsing, validation, and generation in tests and the integration schema fixture. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a generation lifecycle/plugin system to response type generation and uses it to correctly model conditional field selection via GraphQL @include/@skip as nullable in generated result types, plus extends validation to cover directive-argument variables.
Changes:
- Introduces
Grephql.Generation.Pluginlifecycle (normalize → resolve → lower → create) with built-inSkipIncludeand user-configurable:generation_plugins. - Updates type generation to build a
Grephql.Generation.Schematree, run lifecycle hooks, and lower/compile modules from that tree. - Extends variable validation to treat variables referenced in directive arguments as “used” and type-check them against directive arg definitions.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/support/schemas/integration.json | Adds skip/include directives to the integration schema fixture. |
| test/integration_test.exs | Adds integration coverage for @include decoding omitted fields as nil. |
| test/grephql/validator/rules/variables_test.exs | Adds tests for directive-argument variable usage and type mismatch. |
| test/grephql/validator/rules/directives_test.exs | Adds tests for operation-level directives on query/mutation. |
| test/grephql/validator_test.exs | Adds end-to-end validator tests for operation-level directives. |
| test/grephql/type_generator_test.exs | Adds coverage for module ordering and object-parent inline fragment flattening. |
| test/grephql/schema/parser_test.exs | Extends schema directive parsing coverage (QUERY/MUTATION locations). |
| test/grephql/printer_test.exs | Adds printer roundtrip tests for operation directives. |
| test/grephql/parser_test.exs | Adds parsing tests for query/mutation operation directives. |
| test/grephql/generation/skip_include_test.exs | Adds comprehensive generation/plugin tests for @include/@skip nullability behavior. |
| test/grephql/generation/schema_test.exs | Adds tests for Generation.Schema.map_fields/2 traversal behavior. |
| test/grephql/generation/plugin_test.exs | Adds tests for plugin default identity callbacks and overriding. |
| test/grephql/client_module_test.exs | Verifies use Grephql can inject :generation_plugins into generation pipeline. |
| lib/grephql/validator/rules/variables.ex | Collects/type-checks variables used in directive arguments across selections. |
| lib/grephql/type_generator.ex | Refactors generation into normalize/resolve/lower/create pipeline with plugin hooks. |
| lib/grephql/macros.ex | Plumbs @grephql_generation_plugins into compiler invocation. |
| lib/grephql/generation/schema.ex | Introduces the generation tree node struct + map_fields/2. |
| lib/grephql/generation/plugins/skip_include.ex | Adds built-in plugin to mark conditional fields nullable. |
| lib/grephql/generation/plugin.ex | Defines the generation plugin behaviour and default callback implementations. |
| lib/grephql/generation/field.ex | Introduces generation-time field struct and nullability mutation helper. |
| lib/grephql/generation/context.ex | Adds plugin callback context struct (schema, scalar types, fragments). |
| lib/grephql/compiler.ex | Passes :generation_plugins into type/fragment generation. |
| lib/grephql.ex | Adds public :generation_plugins option to use Grephql. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…arents An inline fragment without a type condition is valid GraphQL but raised at generation time on a union/interface parent, since normalize_union_selections/3 (and resolve_union/4) dereferenced a nil type_condition. Untyped inline fragments now hoist their members — directives propagated — into the shared selection rather than being treated as a variant. Addresses Copilot review feedback on #68. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The "legacy path" / "old collect_selections" the comments compared against is deleted by this PR, so the references only confuse a future reader. Kept the load-bearing WHY (rebuilding from resolved so plugin mutations flow through). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Exercise before_normalize / after_normalize / after_lower through a real generate run (only after_resolve was covered in-pipeline before). - Cover the fragment-level no-op literal case (@include(if: true) on an inline fragment leaves inner fields non-null). - Make the generation_plugins end-to-end test assert an observable effect (the plugin drops a field) so it fails if the option is not wired through, rather than only proving generation does not break. - Drop a duplicate assertion and fix a comment that described a missing check; simplify a nested Map.fetch! chain. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Forced nullability is invisible at runtime (Code.Typespec.fetch_types/1 returns :error for Module.create'd generated modules), so the plugin extensibility tests now demonstrate an observable rename (:name -> :display_name, sourced from the original key) instead. Also fixes the skip_include test helper: the capture plugin ran before user plugins, so it never observed their effect — the previous force-nullable assertion passed only because the field was schema-nullable anyway. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fahchen
changed the title
Checks end-to-end that @include(if: $var) makes a non-null field nullable in the generated @type (id: String.t() | nil), while an undirected non-null field stays non-null (name: String.t()). This is assertable because the fixture is a compiled .ex client, so its generated modules have .beam files that Code.Typespec.fetch_types/1 can read — unlike the in-memory modules the other generation tests build at runtime. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@include/@skip, or fields under a conditional fragment) are now generated nullable — the server may omit them even when the schema says non-null.Grephql.Generation.Plugin; stepsnormalize → resolve → lower → createwithafter_<step>hooks).@include/@skipships as the built-inSkipInclude; users register their own via:generation_pluginsonuse Grephql.@include(if: $var)).Design notes
TypeMapper.resolvemap → flatGrephql.Generation.Field→Grephql.Generation.Schematree), not a new descriptor IR.@type.Testing
mix precommitgreen — 654 tests, 0 failures;credo --strictclean; dialyzer 0 errors. Covers@include/@skipon fields, inline fragments, and fragment spreads; no-op literals; union propagation; the untyped-fragment edge case. Nullability is asserted at the IR level, plus one end-to-end test on the generated@type(id: String.t() | nil, via a compiled fixture) and plugin wiring via an observable field rename.🤖 Generated with Claude Code