DataViews Extensibility: Add a hook to allow third-party scripts to register/unregister post type actions #64138
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…egister/unregister post type actions
youknowriad
added
[Feature] Extensibility
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
One small (or not) performance optimization in this PR is that the actions are only registered when needed, they are not globally registered anymore. |
|
Size Change: +4.12 kB (+0.23%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
youknowriad
force-pushed
the
update/post-type-actions-registration
branch
from
031cbe2
to
f41e562
Compare
|
Overall, it’s a common problem that we have with plugins which often run before the editor registers its default handlers, the same applies to block types, block variations. I was thinking again about replicating: add_action( ‘init’, ‘someCallback’ );Today, we offer in the documentation a workaround with wp.domReady( someCallback );Example: removing registered block types. However, it’d definitely help to have it more formalized. It’s only the question whether we introduce specific actions like in this PR, mix that with some general purpose: wp.hooks.addAction( ‘editor/init’, ‘someCallback’ );Or we offer both. Concluding, I’m very much in favor of the proposed approach. I’m also happy to discuss further the overall strategy for other extensibility points that addresses the developer experience hurdles when plugins run code to early by introducing familiar patterns as they have to use in PHP that ensure the code runs at the right time. |
|
Pros here:
Cons:
I think it's the right approach though, we've been accumulating these global registration functions and the limit is showing. |
Can you share more details regarding that aspect? Do you mean the problem that some plugins can run after the Can we avoid it by implementing a similar solution to One good aspect would be that when |
ellatrix
reviewed
Aug 2, 2024
| const { registerPostTypeActions } = unlock( useDispatch( editorStore ) ); | ||
| useEffect( () => { | ||
| registerPostTypeActions( postType ); | ||
| }, [ registerPostTypeActions, postType ] ); |
There was a problem hiding this comment.
I don't love these kinds of effects tbh. Not sure what the alternative is, I guess filtered context wouldn't work.
There was a problem hiding this comment.
This is for me a good thing: Only register the actions and run the filter when needed (when using usePostAction hook), that said, I agree that the code is weird, but I don't know of another pattern that provides the same benefits.
ellatrix
reviewed
Aug 2, 2024
| } ); | ||
| } ); | ||
|
|
||
| doAction( 'core.registerPostTypeActions', Promise.resolve(), postType ); |
There was a problem hiding this comment.
Could our action registration use the same hook?
There was a problem hiding this comment.
I can see two sides here:
- Core code is not special and should use the same thing.
- Adds complexity: figuring out when to register the action, which package and also no special treatment for performance...
ellatrix
approved these changes
Aug 2, 2024
|
The |
97 tasks
I drafted PR to keep the discussion going: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related #61084
What?
In #62052 an API to register and unregister dataviews actions has been implemented. But in order to allow third-party developers to be able to unregister these actions properly we need to offer a hook that third-party can use to "time" their register/unregister function calls properly. This PR does so by introducing the
core.registerPostTypeActionshook to do so.So something like:
Another goal of this PR is to allow us (core) to use the "post type" object to register/unregister actions. For instance "viewPost" action need to only be register for post types with is_viewable as true...
Testing Instructions
Check that the post actions dropdown work properly in both the dataviews and the editor.