Conversation
|
Sorry this took so long to get to @Neychok! Could you refresh this PR against the current state of |
|
@desrosj, just refreshed it. Thank you for looking into this! |
src/wp-includes/blocks.php
Outdated
| ) | ||
| ); | ||
| if ( ! file_exists( $script_asset_path ) ) { | ||
| if ( false === $script_asset_path ) { |
There was a problem hiding this comment.
Also commented on the ticket, but it appears my initial suggestion to use a false === $script_asset_path comparison was incorrect. While realpath() returns explicit false when a file does not exist, wp_normalize_path() will return an empty string when passed false. Seems an empty() check would be more appropriate.
src/wp-includes/blocks.php
Outdated
| /* translators: 1: Field name, 2: Block name. */ | ||
| __( 'The asset file for the "%1$s" defined in "%2$s" block definition is missing.' ), | ||
| __( 'The asset file (%1$s) for the "%2$s" defined in "%3$s" block definition is missing.' ), | ||
| dirname( $metadata['file'] ) . '/' . substr_replace( $script_path, '.asset.php', - strlen( '.js' ) ), |
There was a problem hiding this comment.
Could we avoid having to do this twice? A third variable in addition to $script_handle, and $script_asset_path could store the result of this line before passing to wp_normalize_path( realpath() )above and then to the_doing_it_wrong()`.
There was a problem hiding this comment.
I agree. 749efcf stores the result in a variable for reuse.
src/wp-includes/blocks.php
Outdated
| _doing_it_wrong( | ||
| __FUNCTION__, | ||
| sprintf( | ||
| /* translators: 1: Field name, 2: Block name. */ |
There was a problem hiding this comment.
The translator comment needs to be updated to correctly describe all three placeholders.
|
Working as expected, no fatal error occured. |
Added asset file path in
_doing_it_wrong()notice inregister_block_script_handle()Changed the
! file_exists()check to! file_exists( $script_asset_pathas suggested by desrosjTrac ticket: https://core.trac.wordpress.org/ticket/53566