Skip to content

fix: Allow compatible interface field override with register_graphql_field()#3539

Merged
jasonbahl merged 25 commits into
mainfrom
fix/allow-compatible-interface-field-override
May 21, 2026
Merged

fix: Allow compatible interface field override with register_graphql_field()#3539
jasonbahl merged 25 commits into
mainfrom
fix/allow-compatible-interface-field-override

Conversation

@jasonbahl

@jasonbahl jasonbahl commented Feb 3, 2026

Copy link
Copy Markdown
Collaborator

Fixes #3096

Problem

When using register_graphql_field(), WPGraphQL could throw DUPLICATE_FIELD even when an override was compatible with an inherited interface field.

Example: if an interface defines seo: SeoInterface, overriding Post.seo to PostSeo implements SeoInterface should be valid, but duplicate checks previously happened before compatibility could be confirmed.

Solution

TypeRegistry::register_field() now performs an explicit compatibility check for duplicate overrides that appear to come from interface inheritance:

  1. Resolve the existing duplicate field's underlying type (including wrapped types such as list_of / non_null)
  2. Confirm the existing field is interface-derived
  3. Confirm the overriding object type implements that interface
  4. Allow only verified compatible overrides; otherwise keep DUPLICATE_FIELD

This includes recursion-safe handling and registered-but-lazy-loaded type scenarios.

Correctness Fix Note (Behavioral Change)

This PR intentionally tightens duplicate-field override validation.

  • Before: Some edge cases could be permissive/inconsistent based on type-loading timing.
  • After: Duplicate overrides are only allowed when compatibility is verified.

Downstream plugins that accidentally relied on those permissive edge cases may now see DUPLICATE_FIELD where they previously did not.

We are documenting this as a correctness fix, not a formal breaking change, because it aligns behavior with valid schema/interface rules and preserves intended extension behavior for compatible overrides.

Additional Schema Safety Adjustment

To avoid exposing an invalid root field in environments with no exposed settings:

  • Settings type is only registered when real settings fields exist
  • RootQuery.allSettings is conditionally exposed only when settings fields are available

This avoids synthetic placeholder fields while keeping schema validity.

Tests Added / Updated

Interface override compatibility

  • testIncompatibleInterfaceFieldOverrideStillErrors
  • testCompatibleInterfaceFieldOverrideWithWrappedListInterfaceType
  • testCompatibleInterfaceFieldOverrideWithListOfInterface (based on downstream real-world plugin scenario)
  • Existing duplicate/connection behavior tests preserved and passing

Interface field inheritance cleanup

  • Removed ambiguous TODO/debug-noise in interface inheritance tests and kept assertions definitive

Settings schema behavior

  • testAllSettingsFieldIsHiddenWhenNoSettingsAreExposed

Validation

  • tests/wpunit/TypesTest.php
  • tests/wpunit/InterfaceTest.php
  • tests/wpunit/SettingsQueriesTest.php
  • PHPStan (composer run phpstan -- --memory-limit=2G) for @wpgraphql/wp-graphql

All passing locally in wp-env.

Impact

  • Breaking Changes: None (documented correctness fix)
  • Backward Compatibility: Compatible interface overrides continue to work; invalid/unverifiable duplicate overrides are now consistently rejected
  • Performance: Minimal; added checks run only in duplicate-field registration paths

Related

Adds comprehensive test coverage for issue #3096:
- Test that compatible interface field overrides are allowed
- Test that incompatible overrides still error
- Test that non-interface duplicates still error (preserves existing behavior)
- Test that type modifiers (non_null, list_of) work correctly

These tests will initially fail until the fix is implemented.
…field()

Implements compatibility check in TypeRegistry::register_field() to allow
overriding interface fields with compatible object types that implement
the interface.

When a duplicate field is detected:
- If existing field is callable (from interface) and new field is
  string/array (object type), check if new type implements the interface
- If compatible, allow the override; otherwise throw DUPLICATE_FIELD error
- Uses static flag to prevent infinite recursion during type resolution

Fixes #3096
@codecov

codecov Bot commented Feb 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.13514% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.4%. Comparing base (03fb354) to head (afc1179).

Files with missing lines Patch % Lines
plugins/wp-graphql/src/Registry/TypeRegistry.php 83.6% 22 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##              main   #3539    +/-   ##
========================================
  Coverage     83.4%   83.4%            
- Complexity    5217    5282    +65     
========================================
  Files          286     286            
  Lines        22601   22745   +144     
========================================
+ Hits         18854   18980   +126     
- Misses        3747    3765    +18     
Flag Coverage Δ
wp-graphql-acf-wpunit-twentytwentyfive-single 77.1% <ø> (ø)
wp-graphql-wpunit-twentytwentyfive-multisite 84.5% <85.1%> (+<0.1%) ⬆️
wp-graphql-wpunit-twentytwentyfive-single 84.5% <85.1%> (+<0.1%) ⬆️
wp-graphql-wpunit-twentytwentyone-multisite 84.5% <85.1%> (+<0.1%) ⬆️
wp-graphql-wpunit-twentytwentyone-single 84.5% <85.1%> (+<0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ugins/wp-graphql/src/Type/ObjectType/RootQuery.php 96.2% <100.0%> (+<0.1%) ⬆️
plugins/wp-graphql/src/Type/WPInterfaceTrait.php 85.4% <100.0%> (+0.9%) ⬆️
plugins/wp-graphql/src/Registry/TypeRegistry.php 93.7% <83.6%> (-2.2%) ⬇️

... and 3 files with indirect coverage changes

🚀 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.

Fixes PHPCS PSR2.Files.EndFileNewline.NoneFound error
… guard

Replace static variable with class property for checking_compatibility flag to improve consistency with codebase style and testability.
@jasonbahl

Copy link
Copy Markdown
Collaborator Author

@justlevine more changes coming. Hold off on review.

Add tests that verify interface field overrides work correctly when
overriding with the same type (e.g., Post.field with type Post).

These tests will fail without the fix, demonstrating the bug.
Tests cover:
- Dovid's exact scenario (both register_graphql_field and filter)
- Multiple interfaces
- Page type (not just Post)
- Custom post types
- Filter-only approach

Related to #3540
Fix infinite recursion and DUPLICATE_FIELD errors when overriding
an interface field on the same type (e.g., Post.field with type Post).

The fix:
- Detects when an existing field type is an interface
- Checks if the new type implements that interface
- Allows the override when types are compatible
- Handles both callable and string field types
- Prevents recursion by checking if types are loaded before resolving
- Uses a class property flag to prevent infinite recursion during compatibility checks

This fixes the issue where narrowing an interface field to the same
type that's currently being loaded would cause infinite recursion.

Fixes #3540
@jasonbahl

Copy link
Copy Markdown
Collaborator Author

@justlevine ok, I reworked the tests and had to rework the fix as well

- Fix variable alignment issues
- Fix PHPStan null coalescing issue by using explicit empty check
- Fix else/if structure to use elseif
- Fix indentation issues
…test coverage

- Fix interface field argument merging across inheritance chains
  - Update get_fields_from_implemented_interfaces() to merge arguments
    from multiple interfaces instead of overwriting
  - Ensures arguments from parent interfaces are preserved when
    child interfaces define the same field

- Add comprehensive test coverage for register_graphql_field
  - Test connection field duplicate handling scenarios
  - Test field name formatting with allowFieldUnderscores
  - Test interface field override edge cases
  - Test field registration with array type modifiers
  - Test various field type configurations

- Unskip and fix testInterfaceFieldInheritanceAndMerging
  - Test now verifies that arguments from parent interfaces,
    child interfaces, and object types are properly merged

- Add comprehensive interface documentation
  - Document interface field inheritance
  - Document interface field argument merging feature
  - Include examples and best practices
  - Cover type narrowing and resolver overrides
- Fix interface field argument merging across inheritance chains
- Add PHPStan suppressions for type compatibility in merge_field_args
- Fix Settings type registration to always register (even with empty fields)
- Add placeholder _empty field with TODO for issue #2459
- Fix undefined $schema variables in test methods
- Fix testSchemaIsValid test by ensuring Settings type is always valid
- All wpunit tests passing (1002 tests, 6538 assertions)
- Add test for string type existing field (covers string type check path)
- Add test for different type when loaded (covers loaded type check path)
- Add test for error handling during type resolution
- Update code comment to explain when string type check path is executed
- All tests passing, improving code coverage for TypeRegistry
@jasonbahl

Copy link
Copy Markdown
Collaborator Author

@justlevine I updated the tests to test your specific case. Let me know when you have a chance to test it further. 🙏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates WPGraphQL’s type/field registration behavior to better support narrowing interface-returned fields via register_graphql_field() (avoiding false DUPLICATE_FIELD errors), and expands tests/docs around interface behavior.

Changes:

  • Add compatibility/recursion-handling logic to TypeRegistry::register_field() to permit compatible overrides of interface-derived fields.
  • Update interface field inheritance behavior and add extensive wpunit coverage for interface overrides, connection duplicates, and field-name/type-modifier scenarios.
  • Expand interface documentation and adjust schema/type initialization behavior in a few tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
plugins/wp-graphql/src/Registry/TypeRegistry.php Adds compatibility checks to allow overriding interface fields when types are compatible and attempts to avoid recursion.
plugins/wp-graphql/src/Type/WPInterfaceTrait.php Changes how fields from multiple implemented interfaces are combined (merging args instead of overwriting).
plugins/wp-graphql/src/Type/ObjectType/Settings.php Ensures the Settings type always registers by injecting a placeholder field when empty.
plugins/wp-graphql/tests/wpunit/TypesTest.php Adds many new wpunit tests covering override compatibility, recursion, type modifiers, connection duplicates, etc.
plugins/wp-graphql/tests/wpunit/InterfaceTest.php Reformats/adjusts tests and un-skips (and expands) a test for interface arg merging.
plugins/wp-graphql/docs/interfaces.md Replaces placeholder page with comprehensive interface documentation and examples.

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

Comment thread plugins/wp-graphql/src/Registry/TypeRegistry.php Outdated
Comment thread plugins/wp-graphql/src/Registry/TypeRegistry.php Outdated
Comment thread plugins/wp-graphql/src/Registry/TypeRegistry.php Outdated
Comment thread plugins/wp-graphql/src/Type/ObjectType/Settings.php Outdated
Comment thread plugins/wp-graphql/src/Type/ObjectType/Settings.php Outdated
Comment thread plugins/wp-graphql/tests/wpunit/InterfaceTest.php
Comment thread plugins/wp-graphql/tests/wpunit/InterfaceTest.php Outdated
Comment thread plugins/wp-graphql/tests/wpunit/TypesTest.php Outdated
Comment thread plugins/wp-graphql/tests/wpunit/TypesTest.php Outdated
justlevine added a commit to AxeWP/wp-graphql-gravity-forms that referenced this pull request Feb 15, 2026
@justlevine

Copy link
Copy Markdown
Collaborator

Sadly this isnt working for me downstream.

image

First thing to note, is the schema docs still thinks the field type is from the interface (GfFieldChoice, even though __typename resolves to the correct type (RadioFieldChoice).

When I try to access fields specific to downstream, you can see that validation fails, assumedly because the Duplicate Field check is preventing the application of the narrowed field.

image

You can see the relevant change in this commit: AxeWP/wp-graphql-gravity-forms@a4fd6bc

Hoping it's just because the function doesn't account for list_of/non_null, but it's very noisy and hard for me to tell.

To test on that branch (so you don't need to manually test gf.

nvm use && npm install
npm run wp-env start
npm run wp-env run -- --env-cwd=wp-content/plugins/wp-graphql-gravity-forms cli composer install
npm run wp-env run -- --env-cwd=wp-content/plugins/wp-graphql cli bash

{ then I applied the branch }

npm run tests:codecept -- run tests/wpunit/AddressFieldTest:testGetField -vvv

Comment thread plugins/wp-graphql/src/Type/WPInterfaceTrait.php
Comment thread plugins/wp-graphql/src/Registry/TypeRegistry.php
Comment thread plugins/wp-graphql/src/Registry/TypeRegistry.php Outdated
Comment thread plugins/wp-graphql/tests/wpunit/TypesTest.php
Refactor duplicate-field compatibility checks to only allow verified interface-compatible overrides, including list/non-null and lazy-loaded type scenarios. Add downstream-inspired regression coverage and conditionally expose allSettings only when settings fields exist to keep schema behavior correct without placeholder fields.
Expose the monorepo phpcs directory inside wp-env so plugin lint scripts can resolve shared WPGraphQL sniffs in containerized runs.
Add regression tests for unregistered and non-object override types so duplicate-field compatibility guards remain covered and future refactors don't reopen this behavior.
Add regression tests for loaded and loader-backed interface name resolution paths plus guard behavior for non-interface and empty type-name string scenarios.
…override

When the compat check resolves the override's target type via get_type, the
graphql_get_type filter runs InstrumentSchema which re-enters the
graphql_{Type}_fields filter chain while the outer compat call is still
in-flight. Each callback in the re-entered chain then hit the
checking_compatibility recursion guard, returned false, and logged a
spurious DUPLICATE_FIELD entry even though the outer pass had already
accepted the override. Guard the duplicate-field branch with a fast no-op
when checking_compatibility is set so the outer decision is preserved
without debug noise.

Also broaden the override compatibility check to a second path: when the
existing field's type is a concrete/scalar but the field itself came from
an implemented interface, look up the interface's declared field type via
the implementing ObjectType and accept the override if the wrapping
structure matches and the override's base type is either identical or a
concrete implementation of the interface's base type. This covers
same-type resolver overrides (e.g. Post.related: Post inherited from a
HasSelfRef interface) without loosening protection for unrelated duplicate
registrations.

The register_field filter callback now receives the WPObjectType instance
(filter accepted-args bumped to 2) so the inherited-field lookup can read
implemented interfaces without round-tripping through get_type.
Squiz.Commenting.FunctionComment.SpacingAfterParamType requires the
variable column to align based on the longest parameter type. The
docblocks for is_compatible_interface_field_override and
is_override_compatible_with_interface_field_type added in the previous
commit had a few extra padding spaces, tripping phpcs in CI.
Adds 4 wpunit tests exercising the inherited-field (Path 2) compatibility
branches that were uncovered by patch coverage:

- testCompatibleInterfaceFieldOverrideWithListOfScalar: drives the
  list_of branch through unwrap_field_type_config and
  unwrap_field_type_instance.
- testCompatibleInterfaceFieldOverrideWithNonNullScalar: drives the
  non_null branch through both unwrap helpers.
- testIncompatibleInterfaceFieldOverrideWithMismatchedWrappersStillErrors:
  covers wrapper-mismatch rejection (interface declares list_of String,
  override declares non_null String).
- testCompatibleInterfaceFieldOverrideWhereInheritedFieldTypeIsInterface:
  covers the inherited-interface-typed-field path through
  resolve_interface_name_from_type_name + type_implements_interface.
…fety

Adds four short subsections to interfaces.md so extension authors know
exactly what overrides are accepted by the new compatibility check:

- "Compatible Override Rules": wrappers must match exactly; base type
  must equal the interface's base or implement it.
- "Narrowing Wrapped Interface Field Types": list_of/non_null example
  mirroring the downstream use case from the PR (and the new wpunit
  coverage).
- "Same-Type Overrides": notes the recursion-safe handling that fixes
  issue #3540, with a Post.related example.
- "Behavioral Change Note": flags that previously permissive duplicate
  overrides may now consistently emit DUPLICATE_FIELD when not
  verifiably compatible, framed as a correctness fix.
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.

DX: Can't narrow down interface subtypes on an implementing object with register_graphql_field()

3 participants