-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Plugins: Add scoping functionality to the Plugins API #27438
Conversation
Size Change: +62 B (0%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
I think, there was a similar proposal in the past. Did you check if there are any previous issues and PRs related to this topic? |
I didn't, I will take a look. |
Ah it seems @psealock introduced something similar in #21908 but that PR didn't progress beyond some initial questions. I still think merging this is worthwhile, both solutions are backward compatible, #21908 needs a simple refactor (to move scope to settings maybe) but already has some tests in it. |
Cool, should we wait for @psealock and decide what to do next? I have a related question, what would be the handling if the |
That PluginArea won't render that plugin.
When a PluginArea doesn't define a scope, it would only accept unscoped plugins. |
The current code treats |
Cool, all clear now. Let's make it happen here on in another PR referenced. It's up to you two now. I can help with review 👍 |
Cool, glad to see this idea get traction! One correction on this question:
In #21908 I had an @senadir if you'd like to port over the unit tests I had as well, I'm cool to just go forward with this PR and close mine out. |
There was no scope concept before, so I personally wouldn't mind having:
This way you ensure that using the scope is predictable. The edit post page would have for backward compatibility reasons would have to render two plugin areas, but other pages could decide themselves whether an unscoped plugin area makes sense for them. |
I might also have a counterargument to my previous statement 🤣 We have
I think @jorgefilipecosta will know better how those two concepts could fit together. It might be also that they duplicate each other. Anyway, the point is that it suddenly doesn't feel that clear whether having an unscoped plugin area should render only unscoped plugins. Sorry for bringing more confusion than clarity, but I thought it's an important detail that I completely forgot about. It's also possible that we discussed the scope setting introduction in the settings of the plugin in the past and we didn't proceed further because of this duality. |
The point of having a scope is to avoid accidental rendering of unwanted stuff, taking my case, for example, I only want people who are explicitly registering to my pluginArea to render, people who don't have scope are people not registering to me and therefore prefer to risk having them show up in a wrong context that might be missing dependencies and having them breaking the area. |
For the sake of argument, I can tell that you could also achieve the same effect by adding scopes to the slots you include on the page and ensure that only fills that define the expected scope would render. The question here is what would be more convenient from the perspective of the developers using both sides of this API. Honestly speaking, I have no idea. @mcsf, do you remember if there was a prior discussion on the same topic? |
Given that we already have a scoped slot & fills https://github.com/WordPress/gutenberg/tree/master/packages/interface/src/components/complementary-area?rgh-link-date=2020-12-04T16%3A10%3A03Z#scope I'm not sure having stopped plugin areas provides an added benefit. It seems we are going to the path of slot&fills having the scope, In that case, I guess we don't need scopes in the plugin areas. A third party plugin that wants to implement extensible screens/areas can expose scoped slots? |
scoping for SlotFills seems to solve a very specific need for Gutenberg but doesn't work well for other cases. All of that aside, this doesn't solve the fact that having several parents extensions (Gutenberg, WooCommerce...) owning a pluginArea in the same page would cause clash and increase the risk of having thing render in the wrong place when 3PD tries to integrate into them. |
@senadir, thanks for sharing more details and interesting use cases I haven't considered myself. That's a valid point that since The biggest challenge is how to name and advertise those two scoping concepts. |
We're also going to use scoped Slots in our code, I'm yet to figure out some details but the this is my case, I can use to break down the different:
In this sense, scope for Each plugin would probably define one scoped A PluginArea is the entry point for any code to render, and a Slot is what decides where should that code render. |
I think this discussion helped me to better picture how all the different concepts could fit together. Thank you @senadir. I must admit it was extremely challenging to process all that since I wanted to make sure we not only introduce another prop or setting but we know how to communicate it to those consuming API. I'm on-board with landing this proposal after PR gets updated with:
Separately (another PR it seems), we can discuss how to scope other sites than the edit post one: edit site, edit widgets, etc. Should we have two areas on edit post page? 🤔 What do you think? |
👋 I just wanted to echo the following from Grzegorz's latest comment:
|
2ea8af5
to
5db5854
Compare
17e6593
to
43da253
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.
Code changes look good, backwards compatibility remains intact, and this achieves the goal of only having relevant components added to certain pages.
I still need to compile a dev note for WordPress 5.8 before I land this change. |
Dev note for WordPress 5.8: Scopes in the block editor Plugins APICurrently, when you register a plugin with Plugins API from the There is now a new import { PluginArea, registerPlugin } from '@wordpress/plugins';
import MyPlugin from './my-plugin';
registerPlugin( 'plugin', {
render() {
return <MyPlugin >Renders in the default area</MyPlugin>;
},
} );
registerPlugin( 'plugin-scoped', {
render() {
return <MyPlugin>Renders in the scoped area</MyPlugin>;
},
scope: 'named',
} );
const App = () => {
return (
<>
<PluginArea />
<PluginArea scope="named" />
</>
);
}; |
Description
Currently, when you register a plugin with Plugins API from the
@wordpress/plugins
package, that component is rendered on all pages that use thePluginArea
component. In addition to that, if you renderPluginArea
multiple times, then the same plugin will get rendered multiple times on the same page. The solution for that issue is the concept of scopes. This way, you can have more fine-grained control of where your plugin is going to be displayed.There is now a new
scope
field added to the settings object passed as a second param to theregisterPlugin
function. There is also ascope
prop introduced to thePluginArea
component. A namedPluginArea
would only render plugins registered for the matching scope passed to theregisterPlugin
call. If the scope is not provided, then all plugins without a scope will render as before. It makes this approach fully backward compatible.This PR combines work from #21908 proposed by @psealock.
Testing
The existing functionality should work as before.
npm run test-e2e
should still pass.New unit tests were added that cover new functionality:
npm run test-unit
should pass.Type
This PR adds new functionality to public APi in
@wordpress/plugins
package.