Closed CocoJr closed 5 years ago
Hi thank you for your feedback, this can also be a config parser like graphql or xml or yaml but this one would be optionnal (to doctrine). Doing so we will benefit of realtime modifications, base on doctrine ORM. What do you think of that?
That will be amazing!
I migrate my API to GraphQL, your bundle is very easy-to-use BUT it's very long to configure the type when your application is big (10 more entities, 50+ properties / entity).
What is your plan to make this? Do u need any helps?
Thanks for your quick response, and congratulation for this great bundle :)
Do you want to work on this and submit a PR @CocoJr ?
With pleasure :)
@mcg-web @CocoJr would it be an idea to make it a plugin
or so? in a different repo?
This way we don't have to deal with Doctrine versions or such in this one and also we keep a good separation of concerns.
I'd suggest @CocoJr can create a new repo, we integrate and etc, when it's ready we push it into the overblog namespace!
I know it's more work but I strongly suggest that for the long term maintenance and proper version control!
@renatomefi It's not really a deal with Doctrine, only one more parser called DoctrineParser, which one can parse .php file for the schema
I agree this is just a parser we'll not deal with doctrine since this is optional.
So it's basically reading the properties of a class? Is it considering annotations or so?
Exactly. The deals it's just to use reflexion class to read the annotation to make the appropriote schema, without injecting any other 3rd party bundle.
Maybe we should name it EntityParser or ReflexionClassParser?
That's what I'm thinking, whether it's generic or doctrine related
Maybe we should name it EntityParser or ReflexionClassParser?
@CocoJr How exactly you plan to deal with annotations? Just a string matching or actually using doctrine annotations library to have a proper object type?
Exactly. The deals it's just to use reflexion class to read the annotation to make the appropriote schema, without injecting any other 3rd party bundle.
I think the string parsing is the best solution, because we don't want to inject some doctrine dependency here. I just need to read Column and Relation annotation (it's not really complex to deal with only that), and create new annotation to use full of your work with entity.
Maybe there's something useful here https://github.com/API-Skeletons/zf-doctrine-graphql/blob/master/README.md
Hey ! I'm starting working on it. It's just a first approach of the problem, but it's really powerfull when you already use Doctrine with annotation....
You can follow my work ((it's in progress...) here: https://github.com/CocoJr/GraphQLBundle/tree/0.12
This issue is too complex without Doctrine Annotation Reader. So i just make an extension of your bundle, is better.
Doctrine annotation can be an optional vendor to the bundle no big deal, so only doctrine users will used it with the bundle.
Hum... i don't see how to do that.
Submit the pr with the requirements I'll do the changes later.
Sorry, but i'm looking and honestly i'm totally lost :/ I don't understand how to make the requirements and use the AnnotationParser or other service declared in other 3rd party bundle into this 3rd party bundle.
Ok, i found the good way to be able to parse the annotation with the Doctrine Annotation Reader. I need to add some annotation in addition to use your works, and after that i can submit the P.R.
This will be a good feature ;)
This feature looks great ! :) I tried the annotations branch, but I have a problem (that I don't have with the original from @CocoJr repo). I think it's related to the changes made on the AnnotationParser. I'm trying to use my doctrine entities as graphql type. If I target the same folder for entities & graphql type mapping with annotation, like this:
doctrine:
orm:
auto_generate_proxy_classes: '%kernel.debug%'
naming_strategy: doctrine.orm.naming_strategy.underscore
auto_mapping: true
mappings:
App:
is_bundle: false
type: annotation
dir: '%kernel.project_dir%/src/Entity'
prefix: 'App\Entity'
alias: App
overblog_graphql:
definitions:
show_debug_info: false
schema:
query: RootQuery
mappings:
auto_discover: false
types:
- { type: annotation, dir: "%kernel.project_dir%/src/Entity", suffix: ~ }
I get an error telling me that my "Doctrine\Orm\Mapping\Entity" annotation is not registered.
If it's a different folder, like src/Entity/GraphQL
for the graph mapping, it works.
I'll try to go deeper and let you know.
Ok, it has to do with the way you create an new ClassLoader instead of using the default one in the method getAnnotationReader
of the AnnotationParser.
This doesn't work:
$loader = new ClassLoader();
$loader->setClassMapAuthoritative(true);
This works:
$loader = require __DIR__ . '/../../../../../autoload.php';
It seems that when using your way, the annotation registry is only able to resolve Graphql annotations and not the others ones from ORM for example (so it fails).
Also, AnnotationRegistry::registerFile
is deprecated.
I really like the annotations approach but currently, the behavior is a bit weird.
The annotation parser will map any class in the mapping folder, as long as it has at least one of his property has a @ORM\Column
or @Graphql\Column
.
I think that:
My use case is that I'm using my doctrine entity as graphql type, but I want to be able to manage the field I want to expose in the Graphql Type, and also be able to add property only for the graphql type (like computed properties).
What do you guys have in mind about the annotations feature ? @CocoJr @mcg-web
I'm willing to help on this bundle !
Hi Vincz !
Thanks to develop on this features!
You have to know that in first, i develop that for myself and just testing graphQL with a react's frontend. I totally stop after i see the GraphQL Subscription was not easy to use in PHP.
The annotation @gql\GraphQLType should be mandatory in order to expose a class as Graphql Type. => I think the best approach is to copy the JMS Serializer, and create maybe an annotation like @expose to expose the entity by defaut to GraphQL.
Not sure that the ORM annotations should also be used to determine if a property is exposed in the GraphqlType or not (maybe just use them to auto-guessing the GraphqlType ?). => I'm agree.
It would be great to have more flexibility with the whitelist / blacklist approach. We could choose to expose all properties by default (and a way to exclude some of them if necessary) or expose properties explicitly one by one. => My first answer work here too :D
Thank you @Vincz and @CocoJr for feedback. To be totally honest, my team and I prefer external config files over annotations. Since this is an open source project we trying to make this the easier to use for the larger number of person. @Vincz annotations is still under development for this bundle, we can maybe start by implementing only some gql
annotations totally independent to doctrine ORM annotations in a first place? Contributions or feedback are always welcome anyway :+1:
Hi guys! Thank you for your feedbacks. I already started to work on it from the annotations branch. I'll let you know how it goes. Just for information, what is it you don't like about annotations ? @mcg-web ?
We don't use annotations not because we don't like it but we just prefer the abstraction that comes with externals config files.
Thx @Vincz for your work. I don't have any time to continue this features, because graphQL without Subscription is not really what i need (i can use simple RESTfull with react-apollo-rest for the same result, and continue my REST API in the same way), but i'm sure implementing this solution can be very helpfull for a lot of people. If you need any help, you can contact me, it's just a begining of development
@mcg-web personnal choice, i understand :) But now a lot of people use annotation, for JMS Serializer for exemple, and it's very bad to mix config file with annotation for the entity, complicated to debug :/
Hi @mcg-web & @CocoJr !
So I rewrote the annotations system. I added some tests and updated the documentation.
You can have a look here : https://github.com/Vincz/GraphQLBundle/tree/annotations
At the moment, it's quite simple and it isn't related to Doctrine ORM.
I also added an expression resolver @=resolver_value
that use a method on the value object itself as a resolver (You can check the use on the @Field
annotation here https://github.com/Vincz/GraphQLBundle/blob/annotations/docs/definitions/annotations-reference.md).
So this is the first step.
For the second step, I'd like to be able to auto guess some types based on the Doctrine ORM configuration. But for this, I would need to do the parsing in two times. I would need to parse all the file once and get a map between the GraphQL object and the corresponding PHP class (it would required a modification on the ConfigParser interface and the type extension to have a "preParse" method for example and if it's ok with you @mcg-web). This way, when the objects would be configured, I would be able to guess the GraphQL types from Doctrine ORM annotations (otherwise we cannot if the type name of the class is not the class name itself) or even from reflection with type hint. After that, we would even be able to auto create input object for resolver and even use the Symfony validation on them.
The last thing, I'd like to do, is to be able to define an annotation like @GQL\QueryProvider
or @GQL\MutationProvider
on a class, and it would directly expose root queries or mutations.
It would be great if you would have a few minutes to check this, I really think annotations can add a lot of nice features.
Thanks guy!
Hi @Vincz,
Thanks for this first part! It seem that this makes the annotation implementation a little closer to the others GraphQL config parsers (yaml, xml, GSL). Don't hesitate to open PR on annotations branch if first part is ready to merge, this will help you to don't have to deal with conflicts.
Yes I agree, annotations will add a next and easier way to implement a GraphQL in an existing project.
Hi,
Thanks a lot for your bundle, it's amazing ! However, i see a features not included in your bundle, but really appreciate i think: just one command to generate all types from the entities.
This is an example of command, based on SF4 and not configurable, just for the example: