-
Notifications
You must be signed in to change notification settings - Fork 222
generate a SchemaCompiledEvent when Type files are generated #730
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
Conversation
b2a0bcc
to
4051d2e
Compare
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.
Looks good to me, but I think it could be interesting to do a little more.
Maybe we could also add a preCompile
event with the ability to modify the $configs
before the final compilation.
And we could also add a few parameters to the SchemaCompiledEvent
.
@murtukov what do you think about it ?
Also, a little piece of doc would be great (maybe with your use-case a an example).
PreCompile will lead to bad behaviors since we also use configs in CompilePass, that is not something we want. This will make maintainers work much more difficult. Generator type is just a helper if you want to do something that it doesn't support, you can workaround with custom GraphQL types classes. |
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.
Thanks @mathroc this look good to me but can you provide some documentation and a simple test that verify that the event is well trigger at the right moment, no need of creating a listener, this could be done with simple mock I think.
298ef96
to
954cc0c
Compare
I’ve added a test and some documentation. I noticed something when writing the test, should I only dispatch the event in write mode ? if so, where should I tell the TypeGenerator to write the files in the tests ? |
@mathroc I already did a huge refactoring of the |
954cc0c
to
85a8329
Compare
@murtukov it’s done 👍 It also reminds me that I had to comment out calls to TypeGenerator::addUseStatement, TypeGenerator::addImplement & TypeGenerator::setExpressionLanguage in order to try my changes on master. Should I open an issue to add that to the UPGRADE document? |
fixes #718
usage exemple: