Skip to content
This repository has been archived by the owner on Mar 9, 2024. It is now read-only.

Commit

Permalink
WP-r55888: Script Loader: Improve performance of inlined styles (#272)
Browse files Browse the repository at this point in the history
* WP-r55888: Script Loader: Improve performance of wp_maybe_inline_styles function.

The `wp_maybe_inline_styles` function is called twice on the average page load. On it's second run however, it did not check to see if the style had already been processed on the first run. This resulted in calling `filesize` and `get_file_contents` unnecessarily, which was bad for performance. Now, the loop around the queued styles, checks to see if the source is set to false, meaning it has already been processed. This change also replaces calls to `filesize` with the core function `wp_filesize`, which improves extensibility.

WP:Props spacedmonkey, flixos90, peterwilsoncc, joemcgill.
Fixes https://core.trac.wordpress.org/ticket/58394.

Conflicts:
- tests/phpunit/tests/dependencies/styles.php

---

Merges https://core.trac.wordpress.org/changeset/55888 / WordPress/wordpress-develop@582ddb82f4 to ClassicPress.

* Fix merge conflicts and use css file from repo

* WP-r55909: Script Loader: Add a check to see in style is registered in wp_maybe_inline_styles.

Add a check in `wp_maybe_inline_styles` to check that style is registered before processing items in queue. It is possible that developers may have called `wp_deregister_style`, unregistering style but the style still be in the queue to be processed. Without this check, typing to access the `src` property would result in a notice error.

Follow on from https://core.trac.wordpress.org/changeset/55888.

WP:Props spacedmonkey, flixos90, dd32, kebbet.
See https://core.trac.wordpress.org/ticket/58394.

---

Merges https://core.trac.wordpress.org/changeset/55909 / WordPress/wordpress-develop@559e6cecf4 to ClassicPress.

* Use smaller src css for testing purposes

---------

Co-authored-by: Jonny Harris <[email protected]>
  • Loading branch information
mattyrob and spacedmonkey authored Nov 12, 2023
1 parent fd130e8 commit b57233d
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 4 deletions.
17 changes: 13 additions & 4 deletions src/wp-includes/script-loader.php
Original file line number Diff line number Diff line change
Expand Up @@ -2145,12 +2145,21 @@ function wp_maybe_inline_styles() {

// Build an array of styles that have a path defined.
foreach ( $wp_styles->queue as $handle ) {
if ( wp_styles()->get_data( $handle, 'path' ) && file_exists( $wp_styles->registered[ $handle ]->extra['path'] ) ) {
if ( ! isset( $wp_styles->registered[ $handle ] ) ) {
continue;
}
$src = $wp_styles->registered[ $handle ]->src;
$path = $wp_styles->get_data( $handle, 'path' );
if ( $path && $src ) {
$size = wp_filesize( $path );
if ( ! $size ) {
continue;
}
$styles[] = array(
'handle' => $handle,
'src' => $wp_styles->registered[ $handle ]->src,
'path' => $wp_styles->registered[ $handle ]->extra['path'],
'size' => filesize( $wp_styles->registered[ $handle ]->extra['path'] ),
'src' => $src,
'path' => $path,
'size' => $size,
);
}
}
Expand Down
115 changes: 115 additions & 0 deletions tests/phpunit/tests/dependencies/styles.php
Original file line number Diff line number Diff line change
Expand Up @@ -519,4 +519,119 @@ public function data_styles_with_media() {
),
);
}

/**
* @ticket 58394
*
* @covers ::wp_maybe_inline_styles
*/
public function test_wp_maybe_inline_styles() {
wp_register_style( 'test-handle', '/' . WPINC . '/css/wp-pointer.css' );
wp_style_add_data( 'test-handle', 'path', ABSPATH . WPINC . '/css/wp-pointer.css' );

wp_enqueue_style( 'test-handle' );

wp_maybe_inline_styles();

$this->assertFalse( $GLOBALS['wp_styles']->registered['test-handle']->src, 'Source of style should be reset to false' );

$css = file_get_contents( ABSPATH . WPINC . '/css/wp-pointer.css' );
$this->assertSameSets( $GLOBALS['wp_styles']->registered['test-handle']->extra['after'], array( $css ), 'Source of style should set to after property' );
}

/**
* @ticket 58394
*
* @covers ::wp_maybe_inline_styles
*/
public function test_wp_maybe_inline_styles_dequeue_styles() {
$filter = new MockAction();
add_filter( 'pre_wp_filesize', array( $filter, 'filter' ) );
wp_register_style( 'test-handle', '/' . WPINC . '/css/wp-pointer.css' );
wp_style_add_data( 'test-handle', 'path', ABSPATH . WPINC . '/css/wp-pointer.css' );

wp_enqueue_style( 'test-handle' );

wp_deregister_style( 'test-handle' );

wp_maybe_inline_styles();

$this->assertSame( 0, $filter->get_call_count() );
}

/**
* wp_filesize should be only be called once, as on the second run of wp_maybe_inline_styles,
* src will be set to false and filesize will not be requested.
*
* @ticket 58394
*
* @covers ::wp_maybe_inline_styles
*/
public function test_wp_maybe_inline_styles_multiple_runs() {
$filter = new MockAction();
add_filter( 'pre_wp_filesize', array( $filter, 'filter' ) );
wp_register_style( 'test-handle', '/' . WPINC . '/css/wp-pointer.css' );
wp_style_add_data( 'test-handle', 'path', ABSPATH . WPINC . '/css/wp-pointer.css' );

wp_enqueue_style( 'test-handle' );

wp_maybe_inline_styles();
wp_maybe_inline_styles();

$this->assertSame( 1, $filter->get_call_count() );
}

/**
* @ticket 58394
*
* @covers ::wp_maybe_inline_styles
*/
public function test_test_wp_maybe_inline_styles_missing_file() {
$filter = new MockAction();
add_filter( 'pre_wp_filesize', array( $filter, 'filter' ) );
$url = '/' . WPINC . '/css/invalid.css';
wp_register_style( 'test-handle', $url );
wp_style_add_data( 'test-handle', 'path', ABSPATH . WPINC . '/css/invalid.css' );

wp_enqueue_style( 'test-handle' );

wp_maybe_inline_styles();

$this->assertSame( $GLOBALS['wp_styles']->registered['test-handle']->src, $url, 'Source should not change' );
$this->assertArrayNotHasKey( 'after', $GLOBALS['wp_styles']->registered['test-handle']->extra, 'Source of style not should set to after property' );
$this->assertSame( 1, $filter->get_call_count(), 'wp_filesize should only be called once' );
}

/**
* @ticket 58394
*
* @covers ::wp_maybe_inline_styles
*/
public function test_wp_maybe_inline_styles_no_src() {
wp_register_style( 'test-handle', false );
wp_style_add_data( 'test-handle', 'path', ABSPATH . WPINC . '/css/wp-pointer.css' );

wp_enqueue_style( 'test-handle' );

wp_maybe_inline_styles();

$this->assertFalse( $GLOBALS['wp_styles']->registered['test-handle']->src, 'Source of style should remain false' );
$this->assertArrayNotHasKey( 'after', $GLOBALS['wp_styles']->registered['test-handle']->extra, 'Source of style not should set to after property' );
}

/**
* @ticket 58394
*
* @covers ::wp_maybe_inline_styles
*/
public function test_wp_maybe_inline_styles_no_path() {
$url = '/' . WPINC . '/css/wp-pointer.css';
wp_register_style( 'test-handle', $url );

wp_enqueue_style( 'test-handle' );

wp_maybe_inline_styles();

$this->assertSame( $GLOBALS['wp_styles']->registered['test-handle']->src, $url );
}
}

0 comments on commit b57233d

Please sign in to comment.