Opened 3 years ago
Closed 18 months ago
#56865 closed defect (bug) (fixed)
$args taking effect too late in register_block_type_from_metadata
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.5 | Priority: | normal |
Severity: | normal | Version: | 6.2 |
Component: | General | Keywords: | has-patch has-unit-tests commit |
Focuses: | Cc: |
Description
The register_block_type_from_metadata
function accepts 2 arguments:
$file_or_folder
$args
The $file_or_folder
is a path the the block.json
file, or the folder containing it. $args
is an array of arguments that can be passed to the function in order to provide complimentary data.
The problem right now is that $args
are parsed too late: They get parsed after the scripts/styles have been registered, after all other calculations have been completed, right before we call WP_Block_Type_Registry::get_instance()->register();
.
The problem with the current approach is that $args
are unable to register script/styles, or even be used to take part in all the processing that the register_block_type_from_metadata
function does.
As a side-effect, it's impossible to register a block by passing an array of arguments, without the presence of a block.json
file.
Change History (37)
This ticket was mentioned in PR #3501 on WordPress/wordpress-develop by @aristath.
3 years ago
#1
- Keywords has-patch added
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
2 years ago
This ticket was mentioned in Slack in #core by costdev. View the logs.
2 years ago
#5
@
2 years ago
- Owner set to audrasjb
- Status changed from new to reviewing
As per today's bug scrub, self assigning for review
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
2 years ago
#7
@
2 years ago
- Keywords needs-testing added
This ticket was discussed in the bug scrub.
THe PR need some manual testing.
Additional props @costdev
#8
@
2 years ago
- Keywords needs-unit-tests added
- Milestone changed from 6.2 to 6.3
The PR could also use a unit test to ensure that it's now possible to register a block by passing an array of arguments, without the presence of a block.json
file, and is protected going forward.
As this still needs some work, I'm moving this to the 6.3 milestone. If testing and unit tests are ready before RC1, feel free to pull this ticket back into the 6.2 milestone for commit consideration.
@aristath commented on PR #3501:
2 years ago
#9
Added PHPUnit tests and fixed issues that surfaced from said tests.
This ticket was mentioned in Slack in #core by aristath. View the logs.
2 years ago
#12
@
22 months ago
- Keywords changes-requested added
@aristath (or anyone who can squeeze it into own to-do list) I was unable to apply the patch, can you please look at it and update, and let's try to make it into 6.3 for not changing milestone continuously.
This ticket was mentioned in PR #4778 on WordPress/wordpress-develop by @felipeelia.
22 months ago
#13
This PR is just an attempt to help with the git conflict of #3501
Trac ticket: https://core.trac.wordpress.org/ticket/56865
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
22 months ago
@mukesh27 commented on PR #4778:
22 months ago
#15
Thanks @felipeelia, The unit test going fails that need to address.
@felipeelia commented on PR #4778:
22 months ago
#16
I've pushed bd02e2f to address the problem but I'm actually closing the PR, as I see @aristath already fixed the conflict in #3501. Thanks!
@aristath commented on PR #3501:
22 months ago
#17
Please consider https://github.com/WordPress/wordpress-develop/commit/0bfc842f807582906bc119eb62aff6bd1c1e8dea. This change will revert this change.
Can you please elaborate? How does it revert that change? I don't see it 🤔
@spacedmonkey commented on PR #3501:
22 months ago
#18
Can you please elaborate? How does it revert that change? I don't see it 🤔
In https://github.com/WordPress/wordpress-develop/commit/0bfc842f807582906bc119eb62aff6bd1c1e8dea, I made it so the file_exists is not run on the core blocks. The file exists is pointless as it always exists and ends up I/O. In this PR, you move the file_exists out the if statement, make it run again always, reverting my change.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
22 months ago
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
21 months ago
#21
@
21 months ago
- Milestone changed from 6.3 to 6.4
As per today's bug scrub:
It’s now a bit late for a change like this, and @spacedmonkey requested some other changes in the PR. Let’s move it to 6.4.
This ticket was mentioned in Slack in #core by marybaum. View the logs.
20 months ago
#23
@
20 months ago
On @jorbin's advice we are leaving this in the very capable hands of the two committers who reported and own this ticket.
@spacedmonkey commented on PR #3501:
19 months ago
#25
@aristath Is this good to commit.
@aristath commented on PR #3501:
19 months ago
#26
Yes, it is. 👍
#28
follow-up:
↓ 30
@
19 months ago
Yep thank you for confirming @spacedmonkey, will commit this bugfix next week once I'm back from vacation :)
This ticket was mentioned in Slack in #core by oglekler. View the logs.
19 months ago
#30
in reply to:
↑ 28
@
19 months ago
Replying to audrasjb:
Yep thank you for confirming @spacedmonkey, will commit this bugfix next week once I'm back from vacation :)
@audrasjb are you planning to commit this ahead of 6.5 Beta 5 (Oct 16th) and/or RC1 (Oct 17th)? Is it ready? Still marked with needs-testing
. Not seeing test reports.
This ticket was mentioned in Slack in #core by oglekler. View the logs.
19 months ago
#32
@
19 months ago
This ticket was discussed in today's bug scrub.
@hellofromTonya is reviewing this. Will commit or punt depending on review.
#33
@
19 months ago
- Keywords dev-feedback added
Patch: https://github.com/WordPress/wordpress-develop/pull/3501
Though marked for commit 3 weeks ago (before 6.4 Beta 2), the work was completed and approved this past July.
Given the late stage of the 6.4 cycle with RC1 happening in ~17(ish) hours, I'm not confident enough in that is ready for commit without first shipping in beta releases.
I'm leaning towards punting it to 6.5. But doing a confidence check of those involved.
@costdev @audrasjb @aristath @mukesh27 @spacedmonkey What is your confidence level in the PR based on the above?
If there is high confidence and consensus that this change is ready without (a) side effects or (b) going through beta releases, then it could be considered for commit ahead of RC1.
#34
@
19 months ago
- Keywords needs-refresh added; commit dev-feedback removed
- Milestone changed from 6.4 to 6.5
Chatted in Make/Core slack DM with @costdev who also gave thumbs up to punting. Looking at the patch, there are merged conflicts to resolve. Moving this to 6.5. Also removed the commit
keyword until the merge conflicts are resolved. Also noting would be great to get test reports when the patch is ready.
Trac ticket: https://core.trac.wordpress.org/ticket/56865