-
-
Notifications
You must be signed in to change notification settings - Fork 564
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
Interfaces stopped working #768
Comments
CC @Kingdutch any thoughts? Somehow this was not caught by tests? |
I don't see anything wrong directly. The only hypothesis that I can come up with is that the type or implementation map is being made too early, which causes the implementation check to fail. The simplest method to verify that would be to comment out If that solves the issue then it could possibly be related by the instantiation order of the used |
@Kingdutch just for the info: the error appears in the tests of the library i'm working on (overblog/GraphQLBundle), which is working for years now. I don't think the problem lies with our library, but I tried to comment out the line you told me to and it didn't solve the issue. I'll continue to investigate it and will try to reproduce the error in a more simplified environment. |
@murtukov Do you have a link to where the test is failing? I can't find an update branch/test run in the repo you linked. Being able to look at the failing test can help to see if we can develop a failing test for the graphql-php library as well. |
here is the PR: overblog/GraphQLBundle#786. You can open details of any "CI / PHP" test, they all fail of the same reason. Meanwhile I tried to create an empty project with webonyx/graphql-php 14.5.0 and defined exactly the same schema as described in this issue and it works as expected. |
That seems to indicate that it's not in this library but in the consuming code. Possibly an assumption that is no longer true? I see three test failures. /~https://github.com/overblog/GraphQLBundle/pull/786/checks?check_run_id=1758427937#step:13:1782 /~https://github.com/overblog/GraphQLBundle/pull/786/checks?check_run_id=1758427937#step:13:1799 /~https://github.com/overblog/GraphQLBundle/pull/786/checks?check_run_id=1758427937#step:13:1933 This means that a I think that would explain your spread check failure. Your internal server error is then possibly caused by the strict type check for |
This check is already fixed by me locally, so it isn't an issue RN.
The internal server error is what I described in this issue:
The GraphQLBundle generates PHP classes, that are compatible with the <?php
namespace Overblog\GraphQLBundle\RelayGlobal\__DEFINITIONS__;
use GraphQL\Type\Definition\InterfaceType;
use GraphQL\Type\Definition\Type;
use Overblog\GraphQLBundle\Definition\ConfigProcessor;
use Overblog\GraphQLBundle\Definition\GlobalVariables;
use Overblog\GraphQLBundle\Definition\Type\GeneratedTypeInterface;
/**
* THIS FILE WAS GENERATED AND SHOULD NOT BE EDITED MANUALLY.
*/
final class NodeInterfaceType extends InterfaceType implements GeneratedTypeInterface
{
public const NAME = 'NodeInterface';
public function __construct(ConfigProcessor $configProcessor, GlobalVariables $globalVariables)
{
$config = [
'name' => self::NAME,
'description' => 'Fetches an object given its ID',
'fields' => fn() => [
'id' => [
'type' => Type::nonNull(Type::id()),
'description' => 'The ID of an object',
],
],
'resolveType' => fn($value, $context, $info) => $globalVariables->get('container')->get("overblog_graphql.test.resolver.global")->typeResolver($value),
];
parent::__construct($configProcessor->process($config));
}
} Anyways if the |
Okay I see! Sorry, not familiar with your library, so trying to understand what's happening :D Given that your simplified version does work with the graphql-php library (when not using anything else) that suggests that the error (or the cause of the error in graphql-php) is within the difference. One thing I note is that in your latest example for The reason that I ask is that if I look at what the processor does it seems to expect a plural config array (i.e. |
@Kingdutch I can assure you that config processor is not the reason and it works as expected. I would rather draw your attention to the following. Here is what I see from Xdebug: final class UserType extends ObjectType
{
public function __construct($typeRegistry)
{
$config = [
'name' => 'User',
'fields' => fn() => [
'id' => [
'type' => Type::nonNull(Type::id()),
'resolve' => function ($value, $args, $context, $info) {
// ...
},
],
'name' => Type::string(),
],
'interfaces' => fn() => [
$typeRegistry->get('NodeInterface'),
],
];
parent::__construct($config);
}
} As you can see the This is after all types are generated and the config processors are executed. Hope this helps. |
@Kingdutch ok, I found the problem. The implementing types were invisible, as described here. Don't know why the types weren't invisible before the update tho. Sorry for your time. I'm closing this issue now. |
Glad you found the issue! |
I think the old implementation maybe loaded your types overly eager - which costs performance whenever they are not needed! |
After updating to 14.5.0 I started to see the error:
Simplified schema:
Query:
This is a simplified example, but the thing is it was working before I updated from 14.4.0 to 14.5.0. Seems like it doesn't see anymore that the
User
type implements theNodeInterface
interfaceThe text was updated successfully, but these errors were encountered: