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.

Sign up for GitHub

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

Jump to bottom

fix(gatsby): Only use one redux namespace for plugins #12263

Merged
merged 3 commits into from
Apr 2, 2019
Merged

fix(gatsby): Only use one redux namespace for plugins #12263

merged 3 commits into from
Apr 2, 2019

Conversation

stefanprobst
Copy link
Contributor

Currently we have three redux namespaces for plugins (flattenedPlugins, plugins, apiToPlugins). This PR reduces this to just flattenedPlugins.
Also: use flattenedPlugins.nodeAPIs in api-runner-node and page-hot-reloader.

@stefanprobst stefanprobst requested a review from a team as a code owner March 3, 2019 18:00
@wardpeet
Copy link
Contributor

wardpeet commented Mar 4, 2019

this is cool! Thanks for the cleanup, I still need to test this out. Removing these namespaces does this mean it's a breaking change? I haven't looked really at it.

@stefanprobst
Copy link
Contributor Author

I don't think this was public API (?), so should be good.
apiToPlugins was introduced in #2588; and plugins is used once in page-hot-reloader. Both can easily just use flattenedPlugins.

@stefanprobst
Copy link
Contributor Author

Looks like the one usage in page-hot-reloader also got removed recently in #12671

@stefanprobst
Copy link
Contributor Author

@wardpeet Have you had a chance to look at this? Thanks!

@stefanprobst
Copy link
Contributor Author

Anything missing from this?

@wardpeet wardpeet self-assigned this Mar 26, 2019
@wardpeet
Copy link
Contributor

doing it tomorrow!

@DSchau DSchau added the status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response label Apr 1, 2019
@DSchau DSchau added this to In progress in OSS Roadmap via automation Apr 1, 2019
@wardpeet
Copy link
Contributor

wardpeet commented Apr 2, 2019

Tomorrow took a while but i've tested it and read through the code and it looks 👌

Thanks!

@wardpeet wardpeet merged commit bba69e9 into gatsbyjs:master Apr 2, 2019
OSS Roadmap automation moved this from In progress to Done Apr 2, 2019
@stefanprobst
Copy link
Contributor Author

Cool thanks!

@stefanprobst stefanprobst deleted the remove-apitoplugins-namespace branch April 2, 2019 05:11
@sidharthachatterjee
Copy link
Contributor

Published in gatsby@2.3.5

@pieh pieh mentioned this pull request Apr 2, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wardpeet wardpeet wardpeet approved these changes

Assignees

@wardpeet wardpeet

Labels
status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response
Projects
No open projects
OSS Roadmap
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@stefanprobst @wardpeet @sidharthachatterjee @DSchau