-
Notifications
You must be signed in to change notification settings - Fork 467
fix: recursion issues with interfaces #3100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: recursion issues with interfaces #3100
Conversation
src/Type/WPObjectType.php
Outdated
| * @return array<string, array<string, mixed>> $fields | ||
| */ | ||
| $config['fields'] = function () use ( $config ) { | ||
| $config['fields'] = ! empty( $this->fields ) ? $this->fields : function () use ( $config ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Identical blocks of code found in 2 locations. Consider refactoring.
src/Type/WPInterfaceType.php
Outdated
| $name = ucfirst( $config['name'] ); | ||
| $config['name'] = apply_filters( 'graphql_type_name', $name, $config, $this ); | ||
| $config['fields'] = function () use ( $config ) { | ||
| $config['fields'] = ! empty( $this->fields ) ? $this->fields : function () use ( $config ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Identical blocks of code found in 2 locations. Consider refactoring.
| * @return array<mixed> | ||
| * @throws \Exception | ||
| */ | ||
| protected function get_fields( array $config, TypeRegistry $type_registry ): array { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function get_fields has a Cognitive Complexity of 26 (exceeds 5 allowed). Consider refactoring.
…e implementing Type does not have a description defined on the field already
| * @return array<mixed> | ||
| * @throws \Exception | ||
| */ | ||
| protected function get_fields( array $config, TypeRegistry $type_registry ): array { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function get_fields has a Cognitive Complexity of 29 (exceeds 5 allowed). Consider refactoring.
| * @return array<mixed> | ||
| * @throws \Exception | ||
| */ | ||
| protected function get_fields( array $config, TypeRegistry $type_registry ): array { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function get_fields has a Cognitive Complexity of 30 (exceeds 5 allowed). Consider refactoring.
| * @return array<mixed> | ||
| * @throws \Exception | ||
| */ | ||
| protected function get_fields( array $config, TypeRegistry $type_registry ): array { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method get_fields has 44 lines of code (exceeds 40 allowed). Consider refactoring.
constants.php
Outdated
| // Plugin version. | ||
| if ( ! defined( 'WPGRAPHQL_VERSION' ) ) { | ||
| define( 'WPGRAPHQL_VERSION', '1.23.0' ); | ||
| define( 'WPGRAPHQL_VERSION', '1.24.0' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the version be bumped in this PR going into develop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@josephfusco good catch. I made this change as I needed to test something local using version_compare() but it shouldn't have been committed in this PR. I'll back it out.
…e registered that have the same fieldName, toType and ConnectionTypeName - add isConnectionField identifier to fields registered via WPConnectionType - fix typo in docblock
|
Code Climate has analyzed commit 2e00cd1 and detected 2 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
This Pull Request refactors some under-the-hood logic for how interface fields are applied to Interfaces and Object Types that implement interfaces to make things more performant.
Instead of using
array_merge_recursivethis instead does some diffing on the Implementing Type and the interface(s) being implemented and more efficiently applies any missing fields from the Interface to the implementing Type.For context, below is a summary of the existing functionality this refactor impacts.
Summary of Existing Functionality this Impacts
When implementing an Interface on another Interface or ObjectType, WPGraphQL "automatically" ensures that any fields defined on the Interface(s) are added to the implementing Interface or Object Type if the implementing Type does not already have the field.
This allows for plugins to do things like:
This code will apply the
MyTestInterfaceto thePostType, but will also ensure that thePosttype hasfieldA, which it previously did not.Now I can successfully query like so:
{ posts { nodes { id fieldA } } }