Project separation into multiple dependencies #802
Replies: 15 comments
-
Thanks for bringing this up. Your suggestion makes sense in theory, but I don't see how it will make things easier to maintain. As for me, it is quite the opposite. I guess this is one of the reasons why There were some discussions about similar things in the past, but they never moved forward. I guess it will become possible only if someone expresses a will to work on this and maintain this new package in the foreseeable future. Anyway, I would be happy to hear feedback on this from the community before making any decisions. |
Beta Was this translation helpful? Give feedback.
-
Regarding the discussion of AST adoption, this is a moot point. For example, I use my own AST, which supports the extended syntax of the type system and is not compatible with the original. Also has the ability to extend the syntax using extensions (like this). In any case, I am not ready to discuss this issue now. =) In this case, my proposal concerns only type system, like intermidiate representation. This will allow us to split the parser itself (compilers frontend) from the executable environment (backend of compilers).
In general, I am ready to do this job for webonyx, because made similar actions and only then I thought that it would make sense not to write it))) I think it’s worth asking the authors (@crisu83 @Jalle19 @viniychuk @portey) of other solutions if they are ready to switch to a single type system, like PSR, and in what form this option will suit them. |
Beta Was this translation helpful? Give feedback.
-
P.S. Well, at least ordinary DTO are simpler than classes with logic, like this: https://github.com/webonyx/graphql-php/blob/master/src/Type/Definition/StringType.php#L36-L85 ...аnd if it’s not, why does webonyx still not support the option of repeatable directives?
Or why this interface is not deleted, which is no longer in the reference? |
Beta Was this translation helpful? Give feedback.
-
Hey, just a friendly reminder that this is an open-source project. Nothing stops you from posting an issue or better a PR mentioning problems you face with. Yet I don't see those in this repo. So I guess that's why those artifacts are still there - nobody complained or mentioned them. |
Beta Was this translation helpful? Give feedback.
-
I'm all for trying to reuse code between different implementations. |
Beta Was this translation helpful? Give feedback.
-
It was a rhetorical question)) Just a hint that with code support, not everything is as good as we would like. Well, if there are no objections in general, then it is worth determining how to do this.
interface ObjectTypeInterface extends TypeInterface, CompositeTypeInterface, ...etc
{
public function getName(): string;
public function getDescription(): ?string;
public function getFields(): iterable;
// ...etc
} 1.2) Is it worth implementing abstract classes in this case to simplify implementations? abstract class AbstractObjectType implements ObjectTypeInterface
{
private $name;
public function getName(): string
{
assert(is_string($this->name), 'Invariant violation: Name should be defined');
return $this->name;
}
// ...etc
} 1.3) Or immediately implement basic types without interfaces? class ObjectType implements TypeInterface, CompositeTypeInterface, ...etc
{
private $name;
public function getName(): string
{
assert(is_string($this->name), 'Invariant violation: Name should be defined');
return $this->name;
}
// ...etc
}
Here is a plan of my work. Any objections? |
Beta Was this translation helpful? Give feedback.
-
P.S. I like the implementation from digiaonline. I think that it can be slightly cleaned and used almost in the form in which it is implemented =) |
Beta Was this translation helpful? Give feedback.
-
Please do not rush with it. There is at least one blocker that might make things way more complicated in the future if we choose this approach. We can allow field types to be defined as If we have interfaces for everything this would be a breaking change. Without them, we could at least add Some background on it. And in general, over-exposure of interfaces provokes breaking changes when they could have been avoided otherwise (there was another discussion about it here) |
Beta Was this translation helpful? Give feedback.
-
Imho, filling the types should be vendor specific. That is, whether they were transferred in the constructor configuration or implemented independently - this is not important. ...and it makes sense to make only interfaces, like: interface ObjectTypeInterace ...
{
public function getFields(): iterable;
} In this case Configuration via constructorclass ObjectType implements ObjectTypeInterace
{
private $fields = [];
public function __construct(array $config)
{
$this->fields = $config['fields'] ?? [];
}
public function getFields(): iterable
{
return $this->fields;
}
} Configuration via SDLclass ObjectType implements ObjectTypeInterace
{
// ...
public function getFields(): iterable
{
$parse = fn (string $field) => $this->parser->parse($field);
return \array_map($parse, $this->fields);
}
} Custom Typesclass UserType implements ObjectTypeInterace
{
// ...
public function getFields(): iterable
{
yield 'id' => new Field(new IDType());
yield 'name' => new Field(new NonNullType(new StringType()));
}
} I don’t see such tasks yet, when the interface becomes incompatible. That's why I said that I like the implementation of digia, because it allows you to not depend on implementations, and the interface defines only methods for obtaining data. As example: https://github.com/digiaonline/graphql-php/blob/master/src/Type/Definition/FieldsAwareInterface.php |
Beta Was this translation helpful? Give feedback.
-
Well, in any case, as promised - I sketched the code based on the JS ref version 14.5: The basic set of classes for implementing types based on configs here: @vladar @Jalle19 If you do not mind, I will invite you to this group so that everyone can make complaints about the implementation and correct what they don't like. It is worth considering that this is only a sketch. |
Beta Was this translation helpful? Give feedback.
-
I would like to further discuss some issues. Such types implementation allows you to already implement cross-design extensions that implement additional types. But in this implementation, they do not contain behavior. That is, handler methods like
Should I try to take into account all the ready-made implementations from digia, railt and webonyx and try to implement independent handler methods or the current version is enough for now? |
Beta Was this translation helpful? Give feedback.
-
I have a gut feeling that this whole splitting would cause more trouble than provide benefits. I guess I need a set of clear and specific examples of what it will give the community (vs abstract ideas about how good it could be). So that we could really do some cost/benefit analysis. Also, PoC is totally required for this library as we introduce strict PHPStan checks and I feel that it won't be an easy ride (since utils rely on some fields of type objects like Plus interfaces almost always imply factories for producing specific instances. And injection of those factories. Otherwise, tools like |
Beta Was this translation helpful? Give feedback.
-
Yes, I understand. I think it’s worth the wait when I implement several tools for this set of interfaces, and then it will be possible to say with confidence whether they are needed for webonyx or not. |
Beta Was this translation helpful? Give feedback.
-
Anyways, you didn't mention if you have any specific situation which made you want to have this. There may be different approaches to reach the same goal but it's hard to help without understanding specifics. |
Beta Was this translation helpful? Give feedback.
-
Personally, in my case, as I said, this will simplify the implementation of webonyx executor in my solution. Thus, I will get a ready-made solution for my compiler, where this set of interfaces will act as an implementation of IR (intermediate representation). Now I have already generate code that is fully compatible with these interfaces. It remains to finish the little things, such as namespaces, generics and other). That is, if webonyx supported them, I would not have to do anything to make the solution work, and webonyx would receive support for an alternative SDL compiler, whose lexer, by the way, is twice as fast (see my fork of ur benchmark: https://github.com/SerafimArts/graphql-bench) =))) And webonyx and digiaonline projects can be obtained, for example:
This is what I was able to come up with that can be implemented based on this interfaces. |
Beta Was this translation helpful? Give feedback.
-
Would be good to make the type system a separate dependency so that it can be reused.
The type system should be turned into DTO and isolated from logic (like parsing and serialization).
Reason
The latest ~0.13 version lags far behind the actual code interfaces.
Isolation of the type system and getting rid of logic will make it easier to maintain the current state of the project.
In addition to all this, it will allow to integrate turnkey solutions using webonyx into third-party projects, such as: youshido, railt, digiaonline, etc.
Resume
In the future, it would be nice to separate the type system compiler (SDL) and the executor (GraphQL) into separate repositories (implement a monorepository).
I understand that this issue is rather complicated, but it will allow us to get rid of ecosystem segmentation when each third-party project implements something different, because the rest of the functionality is not required.
Beta Was this translation helpful? Give feedback.
All reactions