Skip to content
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

fix: Made options plugins effective #1178

Conversation

TommyJackson85
Copy link
Contributor

Related Issue

Closes #1177

Your solution

I just moved ...(options.plugins || []) to the bottom of the array.
@Pavel910 Am I right, in that this works becuase the Array is itterated through accendently from the start to the end, with Options essentially overwriting previous data / value changes from the other plugins callbacks?

How Has This Been Tested?

Will test this manually next.

Screenshots (if relevant):

None yet.

@TommyJackson85
Copy link
Contributor Author

I haven't gotten around to testing this yet. Will try tomorrow, if not Sunday. If you need this done asap and feel confident in it though by all means set this for review and merge it!

@Pavel910
Copy link
Collaborator

Pavel910 commented Aug 7, 2020

@TommyJackson85 yes, when this whole array is passed to PluginsContainer it will take care of only using the latest registered plugin, so user-provided plugins will override the once that are built into the app template.

@TommyJackson85 TommyJackson85 marked this pull request as ready for review August 17, 2020 20:38
@TommyJackson85
Copy link
Contributor Author

TommyJackson85 commented Aug 17, 2020

@Pavel910 the setup for testing my changes (from the issue page) is taking me a bit of time to set up, and I'm a little time restrained due to my regular job. So if you feel happy with this change, feel free to merge it anyway! Otherwise I will come back to this later in the week.

@Pavel910
Copy link
Collaborator

@TommyJackson85 thanks for reminding me, totally forgot about this as we are all over the place with the new stuff. Merging this, it's perfectly fine! 🚀 Thanks!

@Pavel910 Pavel910 merged commit ec76ff1 into webiny:master Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom plugins are not reflected on the page builder
2 participants