-
Notifications
You must be signed in to change notification settings - Fork 400
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
feat(mercurius): add plugins option #2538
feat(mercurius): add plugins option #2538
Conversation
Really glad to see this come back and it is, what I would say, a necessity for the uptake in Mercurius usage with Nest. Thanks so much from my humble self @tugascript for the effort. Can't wait to see the hooks PR too. 😀 Scott |
I hope it gets added as a feature. If we can document this functionality in the documentation it would be perfect. 🙌 |
Yeah I have a PR for the docs lined up, I will create that one when this one is merged |
There are few plugings that behaves weirdly. |
This is only for plugins that have to be added after mercurius on the dependency tree, normal fastify plugins, and mercurius-upload will still be loaded in the main file. I will add a warning in the docs about that |
@kamilmysliwiec sorry to tag you, since this is my 3rd time doing this PR, is there any change you want me to do to it? Having access to Mercurius plugins is important for a higher adoption of the framework with NestJS |
I'll check it out asap, hopefully before EOY @tugascript |
👍 Please also check this PR out too, if you could @kamilmysliwiec. #2546 Scott |
Yes this is a wanted feature |
Will this be merged soon? This is a needed feature for us. |
Just use the custom driver from my template. Note the template is for MVP purposes and is not production ready. |
I already have a full project and just need the plugin support its just a simple merge and a lot of people will be happy |
You can use the code on this https://gist.github.com/tugascript/7c7f2df8510303bfbbd6a30d1e3bc421 it has custom drivers based on this PR. The ones on this PR are slightly different, as I minimize the use of state (no changing objectsI, and use only true and false values (no falsy values), aka stricter TypeScript rules |
WE NEED THIS!!! |
I would like to have this aswell since mercurius works much better then apollo and plugin support is essential |
Well this is my last tag @kamilmysliwiec, I messaged you on discord since we are friends there, but it seems you are never online. Could you review this before next weekend? After that I may not be able to do changes till March. I'm not asking for it to be merged right away, I just want to know if there is any change you require for it to be merged before the new version of the nest/graphql package is released just so this feature could go in with it. Plus I need to know if this is the correct branch or there is a official next, the current one seems to be inactive. |
PR looks good to me overall, I can't think of any immediate (required) changes off the top of my head |
Alright, cool when this gets merged, the other PR #2546 will need to be rebased to this branch as there will be conflicts with the utility functions. If in March the Apollo v4 and Mercurius v11 compatibility PRs become stalled I will try to help on them. |
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.
Overall, code looks good, tests look great and actually test the addition, I'm happy with it. Hopefully this can help the feature get merged in sooner rather than later. Thank you everyone for your patience
LGTM |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
There is no plugins options on the Mercurius Adapter.
What is the new behavior?
Adds a plugins option, to be precise an array of the new
MercuriusPlugin
interface.Does this PR introduce a breaking change?
Other information
So this is an update version of an old PR, what is different:
MercuriusDriverPlugin
toMercuriusPlugin
;delete options.plugins
);for of
instead of a normal for loop for readability.