Skip to content
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

Improve loading method for block styles #28358

Merged
merged 45 commits into from Feb 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
913a27b
Inline styles if size is smaller than a threshold
aristath Jan 20, 2021
434b7c6
Add a global pool to avoid too many inline styles
aristath Jan 20, 2021
fcd9389
Merge branch 'master' into try/sustainable-styles
aristath Jan 21, 2021
c207726
change default max pool size to 20k
aristath Jan 22, 2021
137b7cf
Add the original block-name as a param in filters
aristath Jan 22, 2021
2de9db9
Refactor the should-inline calculations
aristath Jan 22, 2021
f734c09
Merge branch 'master' into try/sustainable-styles
aristath Jan 22, 2021
04f05a8
Merge branch 'master' into try/sustainable-styles
aristath Jan 25, 2021
a99f443
Merge branch 'master' into try/sustainable-styles
aristath Jan 25, 2021
0a079f0
Use a transient to cache file sizes
aristath Jan 25, 2021
c7d3dea
Cleanup
aristath Jan 25, 2021
b35e6e7
make sure size is an integer
aristath Jan 25, 2021
c8f22c2
Change threshold to 1500
aristath Jan 25, 2021
6af3170
Remove the total size checks
aristath Jan 25, 2021
60a0be0
Merge branch 'master' into try/sustainable-styles
aristath Feb 1, 2021
4d10158
Merge branch 'master' into try/sustainable-styles
aristath Feb 1, 2021
d22202c
Merge branch 'master' into try/sustainable-styles
aristath Feb 2, 2021
b0ad350
Completely refactor the implementation
aristath Feb 2, 2021
6536df8
Merge branch 'master' into try/sustainable-styles
aristath Feb 2, 2021
253f273
Break the loop when appropriate
aristath Feb 2, 2021
a71ca1f
changed limit to 20k
aristath Feb 2, 2021
ed35fa8
simplify
aristath Feb 2, 2021
bedf8e1
Merge branch 'master' into try/sustainable-styles
aristath Feb 2, 2021
8f681d0
Merge branch 'master' into try/sustainable-styles
aristath Feb 3, 2021
016462e
more tweaks
aristath Feb 3, 2021
74e25fa
Merge branch 'master' into try/sustainable-styles
aristath Feb 4, 2021
1219cae
Use wp_style_add_data, props @gziolo
aristath Feb 4, 2021
d222b39
Simplify condition - props @gziolo
aristath Feb 4, 2021
c493c9a
Add file_exists check back
aristath Feb 4, 2021
bcc0c15
simplify sorting function
aristath Feb 5, 2021
af53a2e
rename var from $stylesheet to $style
aristath Feb 5, 2021
22045e8
typecasting not needed here
aristath Feb 5, 2021
f3f0705
Merge branch 'master' into try/sustainable-styles
aristath Feb 5, 2021
be984b7
rename functions
aristath Feb 5, 2021
c9caa4f
Add basic phpunit tests for the minify function
aristath Feb 5, 2021
91055b4
Merge branch 'master' into try/sustainable-styles
aristath Feb 9, 2021
ba2ef33
Merge branch 'master' into try/sustainable-styles
aristath Feb 11, 2021
ef9216a
Merge branch 'master' into try/sustainable-styles
aristath Feb 12, 2021
fb90254
Merge branch 'master' into try/sustainable-styles
aristath Feb 16, 2021
dba0939
Allow inlining the common.css file if possible
aristath Feb 16, 2021
836e489
improve inline doc
aristath Feb 17, 2021
79b2635
inline docs tweak
aristath Feb 17, 2021
68d9956
Merge branch 'master' into try/sustainable-styles
aristath Feb 17, 2021
716031c
Add wp_style_add_data in docs example
aristath Feb 17, 2021
9309de8
docs
aristath Feb 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -130,6 +130,9 @@ function gutenberg_examples_02_register_block() {
filemtime( plugin_dir_path( __FILE__ ) . 'style.css' )
);

// Allow inlining small stylesheets on the frontend if possible.
wp_style_add_data( 'gutenberg-examples-02', 'path', dirname( __FILE__ ) . '/style.css' );

register_block_type( 'gutenberg-examples/example-02-stylesheets', array(
'apiVersion' => 2,
'style' => 'gutenberg-examples-02',
Expand Down
102 changes: 102 additions & 0 deletions lib/blocks.php
Expand Up @@ -180,6 +180,9 @@ function gutenberg_register_core_block_styles( $block_name ) {
filemtime( gutenberg_dir_path() . $style_path )
);
wp_style_add_data( "wp-block-{$block_name}", 'rtl', 'replace' );

// Add a reference to the stylesheet's path to allow calculations for inlining styles in `wp_head`.
wp_style_add_data( "wp-block-{$block_name}", 'path', gutenberg_dir_path() . $style_path );
}

if ( file_exists( gutenberg_dir_path() . $editor_style_path ) ) {
Expand All @@ -193,6 +196,105 @@ function gutenberg_register_core_block_styles( $block_name ) {
}
}

/**
* Change the way styles get loaded depending on their size.
*
* Optimizes performance and sustainability of styles by inlining smaller stylesheets.
*
* @return void
*/
function gutenberg_maybe_inline_styles() {

$total_inline_limit = 20000;
/**
* The maximum size of inlined styles in bytes.
*
* @param int $total_inline_limit The file-size threshold, in bytes. Defaults to 20000.
* @return int The file-size threshold, in bytes.
*/
$total_inline_limit = apply_filters( 'styles_inline_size_limit', $total_inline_limit );

global $wp_styles;
$styles = array();

// 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'] ) ) {
$block_styles = false;
$styles_size = filesize( $wp_styles->registered[ $handle ]->extra['path'] );

// Minify styles and get their minified size if SCRIPT_DEBUG is not enabled.
if ( ! defined( 'SCRIPT_DEBUG' ) || ! SCRIPT_DEBUG ) {
// Get the styles and minify them by removing comments & whitespace.
$block_styles = gutenberg_get_minified_styles( file_get_contents( $wp_styles->registered[ $handle ]->extra['path'] ) );
// Get the styles size.
$styles_size = strlen( $block_styles );
}

$styles[] = array(
'handle' => $handle,
'path' => $wp_styles->registered[ $handle ]->extra['path'],
'size' => $styles_size,
'css' => $block_styles,
);
}
}

if ( ! empty( $styles ) ) {
// Reorder styles array based on size.
usort(
$styles,
function( $a, $b ) {
return ( $a['size'] <= $b['size'] ) ? -1 : 1;
}
);

/**
* The total inlined size.
*
* On each iteration of the loop, if a style gets added inline the value of this var increases
* to reflect the total size of inlined styles.
*/
$total_inline_size = 0;

// Loop styles.
foreach ( $styles as $style ) {

// Size check. Since styles are ordered by size, we can break the loop.
if ( $total_inline_size + $style['size'] > $total_inline_limit ) {
break;
}

// Get the styles if we don't already have them.
$style['css'] = $style['css'] ? $style['css'] : file_get_contents( $style['path'] );
aristath marked this conversation as resolved.
Show resolved Hide resolved

// Set `src` to `false` and add styles inline.
$wp_styles->registered[ $style['handle'] ]->src = false;
$wp_styles->registered[ $style['handle'] ]->extra['after'][] = $style['css'];

// Add the styles size to the $total_inline_size var.
$total_inline_size += (int) $style['size'];
}
}
}
add_action( 'wp_head', 'gutenberg_maybe_inline_styles', 1 );

/**
* Minify styles.
*
* Removes inline comments and whitespace.
*
* @param string $styles The styles to be minified.
* @return string
*/
function gutenberg_get_minified_styles( $styles ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bug in this function: You can easily see it by checking the "button" block in the "emptytheme". You'll see that the paddings are broken in the frontend.

While we could probably fix this somehow by improving the regex, I wonder if it's actually better to not minify ourselves and instead let the minification tools do the work by:

1- Assuming that the file is already minified (and add it to our setup)
2- Introduce a potential convention: load .min.css if the file is available instead of .css.

I personally lean towards 1 and potentially support for core blocks falling back in the blocks registration to unminified files if SCRIPT_DEBUG is true. (I don't like smart behaviors in general and we should better let the right tools handle the right job :) )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's fixed by #29554 that said I still think we'd better removing the minification from the framework itself.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the bug, #29554 was just merged and fixes it.

I agree however that we should provide minified files, the current ones are OK for debugging but on a real production environment, they are wasteful.

@youknowriad do you know how to do that? I remember I tried to do it in #25220 but ran into issues, mostly because I don't quite understand our tooling and how to use it 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's all in the stylesTransform function we have in the webpack config which in theory should generate minified styles for production code (npm run build) and unmagnified ones for development ( npm run dev)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh... So it already runs as it should, I just always run dev so didn't see that they get minified on production builds 🤦
I'll submit a PR to remove the minify function 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point in the future, we might want to ship both files at the same time and introduce the SCRIPT_DEBUG flag support but it's not only about these blocks styles, it's about all JS and styles in Gutenberg plugin.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point in the future, we might want to ship both files at the same time and introduce the SCRIPT_DEBUG flag support but it's not only about these blocks styles, it's about all JS and styles in Gutenberg plugin.

This is how it works in the WordPress core.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR submitted on #29624 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related issue about shipping .min.* files: #23960

$re1 = '(?sx)("(?:[^"\\\\]++|\\\\.)*+"|\'(?:[^\'\\\\]++|\\\\.)*+\')|/\\* (?> .*? \\*/ )';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's so complex that it could have more unit tests. We can also grow the coverage if any issue is ever discovered.

$re2 = '(?six)("(?:[^"\\\\]++|\\\\.)*+"|\'(?:[^\'\\\\]++|\\\\.)*+\')|\\s*+ ; \\s*+ ( } ) \\s*+|\\s*+ ( [*$~^|]?+= | [{};,>~+-] | !important\\b ) \\s*+|( [[(:] ) \\s++|\\s++ ( [])] )|\\s++ ( : ) \\s*+(?!(?>[^{}"\']++|"(?:[^"\\\\]++|\\\\.)*+"|\'(?:[^\'\\\\]++|\\\\.)*+\')*+{)|^ \\s++ | \\s++ \\z|(\\s)\\s+';

$styles = preg_replace( "%$re1%", '$1', $styles );
return preg_replace( "%$re2%", '$1$2$3$4$5$6$7', $styles );
}

/**
* Complements the implementation of block type `core/social-icon`, whether it
* be provided by core or the plugin, with derived block types for each
Expand Down
1 change: 1 addition & 0 deletions lib/client-assets.php
Expand Up @@ -357,6 +357,7 @@ function gutenberg_register_packages_styles( $styles ) {
filemtime( gutenberg_dir_path() . 'build/block-library/' . $block_library_filename . '.css' )
);
$styles->add_data( 'wp-block-library', 'rtl', 'replace' );
$styles->add_data( 'wp-block-library', 'path', gutenberg_dir_path() . 'build/block-library/' . $block_library_filename . '.css' );

gutenberg_override_style(
$styles,
Expand Down
31 changes: 31 additions & 0 deletions phpunit/class-gutenberg-utils-test.php
Expand Up @@ -127,4 +127,35 @@ public function test_invalid_parameters_set() {
array( 'a' => 2 )
);
}

/**
* Test gutenberg_get_minified_styles().
*/
public function test_gutenberg_get_minified_styles() {
$cases = array(
array(
'in' => '
/**
* Comment
*/
.foo {
bar: 1;
}
aristath marked this conversation as resolved.
Show resolved Hide resolved
',
'out' => '.foo{bar:1}',
),
array(
'in' => '/* Comment */#foo{content:" "; bar: 0;
}',
'out' => '#foo{content:" ";bar:0}',
),
);

foreach ( $cases as $case ) {
$this->assertSame(
gutenberg_get_minified_styles( $case['in'] ),
$case['out']
);
}
}
}