-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Block Editor: Enqueue script modules via enqueue_block_assets action #10597
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -949,4 +949,12 @@ public function print_a11y_script_module_html() { | |||||||||||||||
| . '<div id="a11y-speak-polite" class="a11y-speak-region" aria-live="polite" aria-relevant="additions text" aria-atomic="true"></div>' | ||||||||||||||||
| . '</div>'; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public function clone_without_enqueued_modules(): WP_Script_Modules { | ||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Relatedly, I just learned that PHP 8.5 has |
||||||||||||||||
| $clone = new WP_Script_Modules(); | ||||||||||||||||
| $clone->registered = $this->registered; | ||||||||||||||||
| $clone->dependents_map = $this->dependents_map; | ||||||||||||||||
| $clone->modules_with_missing_dependencies = $this->modules_with_missing_dependencies; | ||||||||||||||||
|
Comment on lines
+954
to
+957
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this be better?
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The result is the same, right? I don't mind either way, if you have reasons to prefer one or the other I'd be happy to know.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mutating the And this is just changing two members instead of three. And these two members feel less internal than the others, meaning we won't need to worry about ensuring other members get copied. And if I understand correctly, resetting the |
||||||||||||||||
| return $clone; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
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.
I learned of a hack/feature in PHP to allow setting private members in PostStatus Slack. This should should have the same effect, including my suggestion below:
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.
That's wild. I can't believe it works and private properties are accessible in the closure.