Backport script loader: enqueue stored block supports styles (refresh) by ockham · Pull Request #3259 · WordPress/wordpress-develop

ockham · 2022-09-15T12:18:26Z

🎼 So Fresh, So Clean 🎶

This is a fork of @ramonjd's #3218, to rebase it (now that #3199 has been merged), and to address feedback by @gziolo.

Original PR desc:

Backporting changes to script-loader that enqueue block support styles stored by the style engine/
See tracking issue: WordPress/gutenberg#43440
Backporting the following changes
Style engine: enqueue block supports styles in Gutenberg gutenberg#42880

(WordPress/gutenberg#4288 has testing instructions.)

Trac ticket: https://core.trac.wordpress.org/ticket/56467

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Backporting block supports filter callback and registrations

ockham · 2022-09-15T12:33:24Z

Unit tests are failing 😕

1) Tests_Theme_wpGetGlobalStylesheet::test_should_enqueue_stored_styles
Registered styles with handle of "core-block-supports" do not match expected value from Style Engine store.
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
Array (
- 0 => '.saruman{color:white;height:100px;border-style:solid;}'
+ 0 => '/**\n
+ * Core styles: block-supports\n
+ */\n
+ .saruman {\n
+ color: white;\n
+ height: 100px;\n
+ border-style: solid;\n
+ }\n
+ '

Edit: Looks like comments and whitespace aren't removed as expected 🤔

ockham · 2022-09-15T18:02:04Z

A similar issue seems to be present over at #3237 (comment) and #3204 (comment).

andrewserong · 2022-09-16T01:45:53Z

A similar issue seems to be present over at #3237 (comment) and #3204 (comment).

I believe the test failure with this one is slightly different to those other two PRs. For the other two PRs, the inline styles tests likely need to be updated to either expect no space between the colon and the value (since the style engine intentionally switched inline style output to remove the space), or for the tests to be more permissive of spaces (i.e. remove spaces between property + colon and value before doing the comparison).

For this one, I think the issue is that (inferring from the test failures) the tests in core appear to be run with SCRIPT_DEBUG set, which switches on prettify for the stored styles output on the followings lines: https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/style-engine/class-wp-style-engine-processor.php#L104, and https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/script-loader.php#L2990

As far as I can tell, for these tests, the SCRIPT_DEBUG value is determined here: https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/default-constants.php#L108 (if I switch that line to $develop_src = false; then the tests pass for me locally).

So, what's the best fix for these particular tests? Given that SCRIPT_DEBUG appears to be set in default-constants.php based on whether or not the current WP version has -src, I was wondering if we should make the tests factor in whether SCRIPT_DEBUG is set to determine which value to expect.

I've opened up (yet another 😅) fork of this changeset in #3262 to try that out. But feel free to close that / copy + paste anything from there if it's helpful.

ramonjd · 2022-09-18T22:50:49Z

So, what's the best fix for these particular tests? Given that SCRIPT_DEBUG appears to be set in default-constants.php based on whether or not the current WP version has -src, I was wondering if we should make the tests factor in whether SCRIPT_DEBUG is set to determine which value to expect.

Thanks for looking into this folks. Sorry I was out of action last week.

Yeah, while I was running these tests locally SCRIPT_DEBUG was activated and I didn't get time to go deeper.

There was a recent change to wp_style_engine_get_stylesheet_from_context however that adds an options arg, through which we can set prettify => true/false. Maybe we could update the function signature of wp_enqueue_stored_styles() to be able to pass that down?

Would that be kosher?

I've opened up (yet another 😅) fork of this changeset in #3262 to try that out. But feel free to close that / copy + paste anything from there if it's helpful.

Thank you @andrewserong !!!

andrewserong · 2022-09-18T23:56:34Z

Maybe we could update the function signature of wp_enqueue_stored_styles() to be able to pass that down?
Would that be kosher?

Passing options to that function sounds like a good idea to me, given that we're supporting options args for most of the public style engine methods, and it'd make the a bit more explicit (e.g. we could test both prettified and non-prettified if we wanted to) 👍

ramonjd · 2022-09-19T00:11:38Z

Passing options to that function sounds like a good idea to me, given that we're supporting options args for most of the public style engine methods, and it'd make the a bit more explicit (e.g. we could test both prettified and non-prettified if we wanted to)

Okey dokey! I'll update this PR, but let's keep #3262 just in case there are doubts. 🙇

ramonjd · 2022-09-19T02:36:16Z

+
+ wp_enqueue_stored_styles();
+
+ $this->assertEquals(


This is a hangover from my original PR.
I think it's better to use assertSame here for string comparisons and assertEquals for objects etc
Suggested change
$this->assertEquals(
$this->assertSame(

ramonjd · 2022-09-19T02:36:23Z

+ 'Registered styles with handle of "core-block-supports" do not match expected value from Style Engine store.'
+ );
+
+ $this->assertEquals(


As above.
Suggested change
$this->assertEquals(
$this->assertSame(

ramonjd · 2022-09-19T02:37:36Z

+ ),
+ );
+
+ // Enqueue some other styles.


I forgot to change this verb tense from imperative to third person present
Suggested change
// Enqueue some other styles.
// Enqueues some other styles.

ramonjd · 2022-09-19T02:46:21Z

There was a recent change to wp_style_engine_get_stylesheet_from_context however that adds an options arg, through which we can set prettify => true/false. Maybe we could update the function signature of wp_enqueue_stored_styles() to be able to pass that down?

@ockham I've added a Gutenberg PR with the proposed changes mentioned in the comment above.

WordPress/gutenberg#44248

Happy to refork this PR with the changes if you don't have the bandwidth to port them across to this PR.

ramonjd · 2022-09-19T05:19:35Z

Happy to refork this PR with the changes if you don't have the bandwidth to port them across to this PR.

Update PR here: #3273

If you're happy with that, we can close this PR

ockham · 2022-09-19T08:38:31Z

Thank you @ramonjd! Closing in favor of #3273.

…sts to 6.1. This changeset backports the following changes: - Implement [WordPress/gutenberg#42880 gutenberg#42880]: Backport script loader: enqueue stored block supports styles - Allow a way to bypass `SCRIPT_DEBUG` in tests. See [WordPress/wordpress-develop#3259 (comment) comment] and the related [WordPress/gutenberg#44248 Gutenberg pull request] Props ramonopoly, gziolo, bernhard-reiter, audrasjb, costdev. See #56467. Built from https://develop.svn.wordpress.org/trunk@54214 git-svn-id: http://core.svn.wordpress.org/trunk@53773 1a063a9b-81f0-0310-95a4-ce76da25c4cd

…sts to 6.1. This changeset backports the following changes: - Implement [WordPress/gutenberg#42880 gutenberg#42880]: Backport script loader: enqueue stored block supports styles - Allow a way to bypass `SCRIPT_DEBUG` in tests. See [WordPress/wordpress-develop#3259 (comment) comment] and the related [WordPress/gutenberg#44248 Gutenberg pull request] Props ramonopoly, gziolo, bernhard-reiter, audrasjb, costdev. See #56467. Built from https://develop.svn.wordpress.org/trunk@54214 git-svn-id: https://core.svn.wordpress.org/trunk@53773 1a063a9b-81f0-0310-95a4-ce76da25c4cd

…sts to 6.1. This changeset backports the following changes: - Implement [WordPress/gutenberg#42880 gutenberg#42880]: Backport script loader: enqueue stored block supports styles - Allow a way to bypass `SCRIPT_DEBUG` in tests. See [WordPress#3259 (comment) comment] and the related [WordPress/gutenberg#44248 Gutenberg pull request] Props ramonopoly, gziolo, bernhard-reiter, audrasjb, costdev. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54214 602fd350-edb4-49c9-b593-d223f7449a82

ockham mentioned this pull request Sep 15, 2022
Backport script loader: enqueue stored block supports styles #3218
Closed

ramonjdand others added 5 commits September 15, 2022 14:25

Initial commit.
db90f69
Backporting block supports filter callback and registrations

Added test
8d10a6e

Remove unused global from test
311df00

Formatting
3c3a710

Move test
23a38f8

ockham force-pushed the add/enqueue-stored-block-supports-styles branch from 3f828d5 to 23a38f8Compare September 15, 2022 12:25

gziolo reviewed Sep 15, 2022
View reviewed changes

Comment threadtests/phpunit/tests/theme/wpGetGlobalStylesheet.php Outdated

Remove obsolete PHPDoc
ca40bbb

This was referenced Sep 15, 2022
Plugin: Backport PHP changes for WordPress 6.1 release WordPress/gutenberg#43440
Closed
Backport Layout block support refactor part 2 #3254
Closed

andrewserong mentioned this pull request Sep 16, 2022
(Alternative) Backport script loader: enqueue stored block supports styles (3rd attempt) #3262
Closed

tellthemachines mentioned this pull request Sep 16, 2022
Sync Gutenberg packages #3154
Closed

ramonjd reviewed Sep 19, 2022
View reviewed changes

ramonjd mentioned this pull request Sep 19, 2022
Block supports: allow overriding prettify options for enqueued CSS WordPress/gutenberg#44248
Merged

This was referenced Sep 19, 2022
Add/enqueue stored block supports styles ramonjd/wordpress-develop#1
Closed
Backporting block supports filter callback and registrations and tests #3273
Closed

ockham closed this Sep 19, 2022

ockham deleted the add/enqueue-stored-block-supports-styles branch September 19, 2022 08:38

Navigation Menu

Uh oh!

Backport script loader: enqueue stored block supports styles (refresh) - #3259

Backport script loader: enqueue stored block supports styles (refresh)#3259
ockham wants to merge 6 commits into
WordPress:trunkfrom
ockham:add/enqueue-stored-block-supports-styles

ockham commented Sep 15, 2022

Uh oh!

Uh oh!

ockham commented Sep 15, 2022•
edited
Loading

Uh oh!

ockham commented Sep 15, 2022

Uh oh!

andrewserong commented Sep 16, 2022

Uh oh!

ramonjd commented Sep 18, 2022•
edited
Loading

Uh oh!

andrewserong commented Sep 18, 2022

Uh oh!

ramonjd commented Sep 19, 2022

Uh oh!

ramonjdSep 19, 2022

Uh oh!

ramonjdSep 19, 2022

Uh oh!

ramonjdSep 19, 2022

Uh oh!

ramonjd commented Sep 19, 2022

Uh oh!

ramonjd commented Sep 19, 2022

Uh oh!

ockham commented Sep 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

ockham commented Sep 15, 2022

Backporting the following changes

Uh oh!

Uh oh!

ockham commented Sep 15, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ockham commented Sep 15, 2022

Uh oh!

andrewserong commented Sep 16, 2022

Uh oh!

ramonjd commented Sep 18, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewserong commented Sep 18, 2022

Uh oh!

ramonjd commented Sep 19, 2022

Uh oh!

ramonjdSep 19, 2022

Choose a reason for hiding this comment

Uh oh!

ramonjdSep 19, 2022

Choose a reason for hiding this comment

Uh oh!

ramonjdSep 19, 2022

Choose a reason for hiding this comment

Uh oh!

ramonjd commented Sep 19, 2022

Uh oh!

ramonjd commented Sep 19, 2022

Uh oh!

ockham commented Sep 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ockham commented Sep 15, 2022•
edited
Loading

ramonjd commented Sep 18, 2022•
edited
Loading