CSS transformation: Fix mangled selectors by switching to a different postcss plugin for prefixing #63972
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
talldan
added
[Type] Bug
talldan
changed the title
|
Size Change: -943 B (-0.05%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
talldan
commented
Jul 26, 2024
talldan
changed the title
|
Here's a diff of the transformed css (comparing trunk and this PR): DiffStyles A 381c381
< .editor-styles-wrapper .has-body-font-family {
---
> .has-.editor-styles-wrapper-font-family {Styles B 25c25
< .editor-styles-wrapper .has-global-padding :where(:not(.alignfull.is-layout-flow) > .has-global-padding:not(.wp-block-block, .editor-styles-wrapper .alignfull)) {
---
> .editor-styles-wrapper .has-global-padding :where(:not(.alignfull.is-layout-flow) > .has-global-padding:not(.wp-block-block, .alignfull)) {
30c30
< .editor-styles-wrapper .has-global-padding :where(:not(.alignfull.is-layout-flow) > .has-global-padding:not(.wp-block-block, .editor-styles-wrapper .alignfull))>.alignfull {
---
> .editor-styles-wrapper .has-global-padding :where(:not(.alignfull.is-layout-flow) > .has-global-padding:not(.wp-block-block, .alignfull))>.alignfull {
68c68
< .editor-styles-wrapper .is-layout-flow> :first-child {
---
> .editor-styles-wrapper :where(.is-layout-flow > :first-child) {
72c72
< .editor-styles-wrapper .is-layout-flow> :last-child {
---
> .editor-styles-wrapper :where(.is-layout-flow > :last-child) {
76c76
< .editor-styles-wrapper .is-layout-flow>* {
---
> .editor-styles-wrapper :where(.is-layout-flow > *) {
81c81
< .editor-styles-wrapper .is-layout-constrained> :first-child {
---
> .editor-styles-wrapper :where(.is-layout-constrained > :first-child) {
85c85
< .editor-styles-wrapper .is-layout-constrained> :last-child {
---
> .editor-styles-wrapper :where(.is-layout-constrained > :last-child) {
89c89
< .editor-styles-wrapper .is-layout-constrained>* {
---
> .editor-styles-wrapper :where(.is-layout-constrained > *) {
94c94
< .editor-styles-wrapper .is-layout-flex {
---
> .editor-styles-wrapper :where(.is-layout-flex) {
98c98
< .editor-styles-wrapper .is-layout-grid {
---
> .editor-styles-wrapper :where(.is-layout-grid) {
155c155
< .editor-styles-wrapper .is-layout-flex> :is(*, .editor-styles-wrapper div) {
---
> .editor-styles-wrapper .is-layout-flex> :is(*, div) {
163c163
< .editor-styles-wrapper .is-layout-grid> :is(*, .editor-styles-wrapper div) {
---
> .editor-styles-wrapper .is-layout-grid> :is(*, div) {
227c227
< .editor-styles-wrapper :where(.wp-element-button, .editor-styles-wrapper .wp-block-button__link) {
---
> .editor-styles-wrapper :where(.wp-element-button, .wp-block-button__link) {
245c245
< .editor-styles-wrapper :where(.wp-element-button:hover, .editor-styles-wrapper .wp-block-button__link:hover) {
---
> .editor-styles-wrapper :where(.wp-element-button:hover, .wp-block-button__link:hover) {
251c251
< .editor-styles-wrapper :where(.wp-element-button:focus, .editor-styles-wrapper .wp-block-button__link:focus) {
---
> .editor-styles-wrapper :where(.wp-element-button:focus, .wp-block-button__link:focus) {
259c259
< .editor-styles-wrapper :where(.wp-element-button:active, .editor-styles-wrapper .wp-block-button__link:active) {
---
> .editor-styles-wrapper :where(.wp-element-button:active, .wp-block-button__link:active) {
264c264
< .editor-styles-wrapper :where(.wp-element-caption, .editor-styles-wrapper .wp-block-audio figcaption, .editor-styles-wrapper .wp-block-embed figcaption, .editor-styles-wrapper .wp-block-gallery figcaption, .editor-styles-wrapper .wp-block-image figcaption, .editor-styles-wrapper .wp-block-table figcaption, .editor-styles-wrapper .wp-block-video figcaption) {
---
> .editor-styles-wrapper :where(.wp-element-caption, .wp-block-audio figcaption, .wp-block-embed figcaption, .wp-block-gallery figcaption, .wp-block-image figcaption, .wp-block-table figcaption, .wp-block-video figcaption) {
292c292
< .editor-styles-wrapper .wp-block-buttons-is-layout-flow> :first-child {
---
> .editor-styles-wrapper :where(.wp-block-buttons-is-layout-flow > :first-child) {
296c296
< .editor-styles-wrapper .wp-block-buttons-is-layout-flow> :last-child {
---
> .editor-styles-wrapper :where(.wp-block-buttons-is-layout-flow > :last-child) {
300c300
< .editor-styles-wrapper .wp-block-buttons-is-layout-flow>* {
---
> .editor-styles-wrapper :where(.wp-block-buttons-is-layout-flow > *) {
305c305
< .editor-styles-wrapper .wp-block-buttons-is-layout-constrained> :first-child {
---
> .editor-styles-wrapper :where(.wp-block-buttons-is-layout-constrained > :first-child) {
309c309
< .editor-styles-wrapper .wp-block-buttons-is-layout-constrained> :last-child {
---
> .editor-styles-wrapper :where(.wp-block-buttons-is-layout-constrained > :last-child) {
313c313
< .editor-styles-wrapper .wp-block-buttons-is-layout-constrained>* {
---
> .editor-styles-wrapper :where(.wp-block-buttons-is-layout-constrained > *) {
318c318
< .editor-styles-wrapper .wp-block-buttons-is-layout-flex {
---
> .editor-styles-wrapper :where(.wp-block-buttons-is-layout-flex) {
322c322
< .editor-styles-wrapper .wp-block-buttons-is-layout-grid {
---
> .editor-styles-wrapper :where(.wp-block-buttons-is-layout-grid) {
326c326
< .editor-styles-wrapper :where(.wp-block-calendar table, .editor-styles-wrapper .wp-block-calendar th) {
---
> .editor-styles-wrapper :where(.wp-block-calendar table, .wp-block-calendar th) {
436c436
< .editor-styles-wrapper :where(.wp-block-post-comments-form textarea, .editor-styles-wrapper .wp-block-post-comments-form input) {
---
> .editor-styles-wrapper :where(.wp-block-post-comments-form textarea, .wp-block-post-comments-form input) {
522c522
< .editor-styles-wrapper :where(.wp-block-post-featured-image img, .editor-styles-wrapper .wp-block-post-featured-image .block-editor-media-placeholder, .editor-styles-wrapper .wp-block-post-featured-image .wp-block-post-featured-image__overlay) {
---
> .editor-styles-wrapper :where(.wp-block-post-featured-image img, .wp-block-post-featured-image .block-editor-media-placeholder, .wp-block-post-featured-image .wp-block-post-featured-image__overlay) {
586c586
< .editor-styles-wrapper :where(.wp-block-quote.has-text-align-right.is-style-plain, .editor-styles-wrapper .rtl .is-style-plain.wp-block-quote:not(.has-text-align-center):not(.has-text-align-left)) {
---
> .editor-styles-wrapper :where(.wp-block-quote.has-text-align-right.is-style-plain, .rtl .is-style-plain.wp-block-quote:not(.has-text-align-center):not(.has-text-align-left)) {
592c592
< .editor-styles-wrapper :where(.wp-block-quote.has-text-align-left.is-style-plain, .editor-styles-wrapper:not(.rtl) .is-style-plain.wp-block-quote:not(.has-text-align-center):not(.has-text-align-right)) {
---
> :root :where(.wp-block-quote.has-text-align-left.is-style-plain, .editor-styles-wrapper:not(.rtl) .is-style-plain.wp-block-quote:not(.has-text-align-center):not(.has-text-align-right)) {
604c604
< .editor-styles-wrapper :where(.wp-block-search .wp-block-search__label, .editor-styles-wrapper .wp-block-search .wp-block-search__input, .editor-styles-wrapper .wp-block-search .wp-block-search__button) {
---
> .editor-styles-wrapper :where(.wp-block-search .wp-block-search__label, .wp-block-search .wp-block-search__input, .wp-block-search .wp-block-search__button) {
614c614
< .editor-styles-wrapper :where(.wp-block-search .wp-element-button, .editor-styles-wrapper .wp-block-search .wp-block-button__link) {
---
> .editor-styles-wrapper :where(.wp-block-search .wp-element-button, .wp-block-search .wp-block-button__link) {There's a pretty obvious issue in the first diff where the replacement of 381c381
< .editor-styles-wrapper .has-body-font-family {
---
> .has-.editor-styles-wrapper-font-family {This also doesn't look right: 592c592
< .editor-styles-wrapper :where(.wp-block-quote.has-text-align-left.is-style-plain, .editor-styles-wrapper:not(.rtl) .is-style-plain.wp-block-quote:not(.has-text-align-center):not(.has-text-align-right)) {
---
> :root :where(.wp-block-quote.has-text-align-left.is-style-plain, .editor-styles-wrapper:not(.rtl) .is-style-plain.wp-block-quote:not(.has-text-align-center):not(.has-text-align-right)) { |
talldan
force-pushed
the
fix/non-iframed-selectors
branch
from
280559d
to
3ce3370
Compare
|
Flaky tests detected in 3ce3370. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10142596352
|
talldan
changed the title
:where styles
talldan
force-pushed
the
fix/non-iframed-selectors
branch
from
3ce3370
to
bf126e6
Compare
talldan
force-pushed
the
fix/non-iframed-selectors
branch
from
bf126e6
to
0a8062f
Compare
talldan
changed the title
:where styles
talldan
commented
Jul 31, 2024
| @@ -208,7 +208,24 @@ describe( 'transformStyles', () => { | |||
| '.my-namespace' | |||
| ); | |||
|
|
|||
| expect( output ).toMatchSnapshot(); | |||
| expect( output ).toEqual( [ input ] ); | |||
There was a problem hiding this comment.
Snapshot was unnecessary (and I think is also not needed for quite a lot of these tests), this just needs to test that the output is the same as the input.
This was referenced Aug 1, 2024
Closed
2 tasks
|
Closing in favour of #64458 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #63829
What?
Tries switching from
postcss-prefixwraptopostcss-prefix-selectorfor transforming stylesWhy?
postcss-prefixwrapis currently mangling compound:whereselectors - Non-iframed Editor: Compound selectors within :where are not prefixed correctlyI've reported it upstream in dbtedman/postcss-prefixwrap#515 and can try making a PR on the repo if one hasn't by the time I get to it.
The issue still needs to be fixed in Gutenberg and core, so this PR should do that.
How?
Switches from postcss-prefixwrap to postcss-prefix-selector. Why does this make a difference?
I had a look at the codebases for the two.
postcss-prefixwrapseems to take the whole selector string, and then call the stringsplit( ',' )method to handle compound selectors:https://github.com/dbtedman/postcss-prefixwrap/blob/c2636cdaa8381741df1b2b553d15a82d1e674802/src/internal/domain/CSSRuleWrapper.ts#L20-L46
This results in the selector being split incorrectly and prefixing within a
whereselector.postcss-prefix-selectoron the other hand seems to use theselectorsproperty provided by the AST which I'd expect to be a lot more advanced in how it splits a selector (it should align pretty closely with the CSS spec):https://github.com/RadValentin/postcss-prefix-selector/blob/86cb9b9dd50caef6e075ac88f9d716975c68a597/index.js#L38
There are a couple of features not offered by the new dependency:
It does have a
transformcallback that I've used to implement these features so that there's parity withtrunk.Testing Instructions
.editor-styles-wrapper :where(.wp-element-button, .editor-styles-wrapper .wp-block-button__link).editor-styles-wrapperwithin thatScreenshots or screencast