Use css-tree for block-editor style transformations (3). #25514

strarsis · 2020-09-21T17:41:34Z

@youknowriad, @oxyc, @noahtallen: Note: This PR supersedes the older PRs (css-tree-transform-styles and css-tree-transform-styles-2) with changes on top of latest upstream master. Special thanks @oxyc for his fixes for the code in this PR.

Description

This PR adds the CSSTree parser for wrapping the theme styles into a styles wrapper and WHATWG URL for URL manipulation of URLs in styles.

How has this been tested?

I tested this on my local WordPress site.

Types of changes

Because a custom CSS parser is currently used, parse errors can occur, as in url([...]url([...])[...]) causes styles wrapping to fail completely (although valid CSS) #21569.
CSSTree is a native JavaScript CSS parser that is already used in other major projects like svgo and csso, hence it should handle all CSS fast and well.
CSSTree can also generate sourcemaps and even reuse existing sourcemaps. With some API changes it should be possible to add sourcemap support in Gutenberg, which means that developers can still see from where the injected styles came from
(PoCs here: https://codepen.io/strarsis/pen/RwWRqZv and also https://codesandbox.io/s/busy-williamson-r1frp). (Not implemented with this PR, but CSSTree adds the foundation for it).
WHATWG URL for manipulating URLs.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.

Use WHATWG URL for block-editor style URL manipulations.

strarsis · 2020-09-21T18:02:23Z

The CI linter task complains: /home/runner/work/gutenberg/gutenberg/build/block-editor/index.js 1:5821 error Parsing error: The keyword 'const' is reserved
So const is not allowed? Is this an issue with upstream master?

youknowriad

The linter is complaining because the library you're including (css-tree probably) is not transpiled so it leaves "const" in the built files which don't work on IE11. I'm not certain how to solve this though.

packages/block-editor/src/utils/transform-styles/transforms/url-rewrite.js

youknowriad · 2020-09-22T08:47:16Z

packages/block-editor/package.json

@@ -54,10 +55,11 @@
 		"@wordpress/wordcount": "file:../wordcount",
 		"classnames": "^2.2.5",
 		"css-mediaquery": "^0.1.2",
+		"css-tree": "^1.0.0-alpha.39",


Why is this using an "alpha" version? can we use the latest stable instead?

Yes, this doesn't make sense, css-tree is in use for many years and very stable, see this issue csstree/csstree#134 for having it finally published at least as a beta release now.

packages/block-editor/package.json

Use normal function type for trimOutmostChars.

strarsis · 2020-09-22T11:49:01Z

@youknowriad: The css-tree package ships with a dist build that can also be required instead.
I require that instead, which should solve the linter/browser compat issue.

packages/block-editor/package.json

packages/block-editor/src/utils/transform-styles/test/traverse.js

packages/block-editor/src/utils/transform-styles/transforms/wrap.js

…ires.

youknowriad · 2020-09-22T12:30:36Z

I've been looking at the bundle-size impact of this PR. See the comment here #25533 (comment)

Unfortunately, it's way too big. +124 kB is 9% of all the JS of Gutenberg for a single dependency. Do you think there's any way to improve that, maybe by trying to load only aa subset of css-tree?

strarsis · 2020-09-22T12:39:35Z

@youknowriad: The linter still complains about const, even with dist. I could switch back to non-dist and see whether the file size goes down. I can't find a const in the file at the position the linter complains about, so apparently the build results diverge for me and the CI/linter environment.

strarsis · 2020-09-22T14:18:02Z

@youknowriad: Have the sizes changed now favourably?
Would it be possible to set up the bundler to transpile the css-tree dependency to ES5? For const, just var is used.

youknowriad · 2020-09-23T08:17:02Z

Looks like the size diff is pretty similar regardless 🤔
I didn't try compiling the dependency yet, I guess we might need to unexclude it from webpack config and potentially babel.

strarsis · 2021-08-18T10:59:08Z

Of course, using a dedicated browser API for this would be great, but sadly it isn't supported at all yet (notably the Parser API): https://ishoudinireadyyet.com/

strarsis · 2022-02-22T15:39:00Z

Hm, what about using the native Browser API and a CSS selector parser (as there is no Browser API for this yet) to achieve this?
PoC: https://codepen.io/strarsis/pen/MWmMQgy
This uses the native CSS parser/CSSOM of the browser.

zaguiini · 2023-03-13T22:46:45Z

This library is indeed a little bit heavy. Tree-shaking (picking some methods directly from the lib folder) reduces it to ~60KB but still, I think we can get something better than that.

https://github.com/adobe/css-tools is promising -- it has a parse function that returns an AST and stringify that returns the compiled CSS. Maybe we could use that instead? The parse function weighs about 10KB, and so does stringify. Maybe we can give it a go.

zaguiini · 2023-03-13T23:25:23Z

packages/block-editor/src/utils/transform-styles/transforms/url-rewrite.js

-function isRemotePath( filePath ) {
-	return /^(?:https?:)?\/\//.test( filePath );
-}
+import { URL } from 'whatwg-url';


I don't think this dependency is necessary anymore. URL has achieved a good level of browser standardization.

Yes, good point. Browser support is basically 100% now. NodeJS supports it, too.

strarsis · 2023-03-13T23:25:26Z

Picking methods directly looks promising, hopefully this shaves off enough bytes to make it acceptable.

strarsis · 2023-03-14T01:42:16Z

Alright, I rebase and resolve all conflicts to get this up to date. Then I clean things up and try to get the size as small as possible.

zaguiini · 2023-03-17T16:06:22Z

I managed to shave some bytes by cherry-picking the css-parse imports on a draft branch. However, it's still a huge price to pay:

trunk: 668kb
non-cherry-picked css-tree: 825kb
cherry-picked css-tree: 729kb

~~According to Bundlephobia, @adobe/css-tools would add 12kb to the trunk bundle size, and that's unminified -- it goes to 3.4kb once minzipped.~~

~~It's not a drop-in replacement, though, so a bigger rewrite would be necessary.~~

EDIT: Surprisingly, we're actually using @adobe/css-tools on trunk! It's a fork of reworkcss/css and that's the code in packages/block-editor/src/utils/transform-styles/ast, so it won't really fix our problem.

I tried using lightningcss but you need to do that top-level await madness in order to kick off the WASM download (which, btw, is 8 MB). Other options are regex-based and they might have the same problems (if not more) that the implementation we currently have.

I think we'll need to bite the bullet here... Considering the improvements and the reliability of css-tree, I'd say it's OK to have a 9.1% increase in bundle size if that means CSS transformations and inlining will work properly. That's my 2 cents, though. Interested in hearing other maintainers' thoughts @youknowriad @ellatrix

youknowriad · 2023-03-20T09:27:36Z

Personally, I would have been ok with the bundle size change, if the fixes are for all users. The reality though is that block themes use iframes and there's a goal to actually move to iframes for everyone which means we'll be paying an extra price for code that is not used for most users. WDYT?

kraftner · 2023-03-20T09:36:02Z

I think realistically a lot of sites will not run on block themes for quite some time (for various, valid reasons) so this is still very much worth fixing. Especially since this is not some kind of new feature but a bug.
All even more so since this issue will probably only get worse as new CSS features are constantly being introduced. Here is one recent example with @layer

strarsis · 2023-03-20T09:42:08Z

@kraftner: CSS Layers are supported by the css-tree parser though.

@youknowriad, @kraftner, @zaguiini; @dsturm:
There is an alternative though, that leverages the existing browser APIs to parse and process the styles, see this working example:
https://codepen.io/strarsis/pen/MWmMQgy
It has an extremely low footprint, should be very fast (as it uses the native browser parsing capabilities) and what the browser can parse, it can parse and process, too.
Its only dependency is CSSwhat as there is no browser API for selector-level CSS parsing yet.

kraftner · 2023-03-20T09:45:48Z

@kraftner: CSS Layers are supported by the css-tree parser though.

Okay, sorry, I didn't actually check this myself. But I think the general assumption that an outdated, broken parser will inevitably only continue to have more bugs when new CSS features get introduced over time still stands. And since WP is strong on back compat this will become a worsening problem for many years to come.

@youknowriad, @kraftner, @zaguiini: There is an alternative though, which uses the existing browser APIs to parse and process the styles, see this working example: https://codepen.io/strarsis/pen/MWmMQgy It has an extremely low footprint, should be very fast (as it uses the native browser parsing capabilities) and what the browser can parse, it can parse and process, too.

No personal opinion on the how, you're much more competent on that @strarsis 😄

strarsis · 2023-03-20T09:47:31Z

@kraftner: I am also unhappy with the state of JS CSS parsers and the ongoing addition of new CSS stuff that has to be supported. This is not a perfect solution of course, this should rather fix the most blocking issues with CSS parsing. The only reason for why I picked css-tree in the first place was that it appears to be the most maintained parser right now. It is quite big, which is its greatest downside (beta versioning was another one). If a better CSS parser can be found, I would gladly use that one instead. With a halfway decent AST (and stringification of course) basically any parser could be used.

youknowriad · 2023-03-20T10:04:14Z

@strarsis What's the downsides of your browser based parsing? Seems like the best option for me so far based on your description?

strarsis · 2023-03-20T10:14:45Z

@strarsis What's the downsides of your browser based parsing?

The browser parsers could be subtly different, that's the only real downside I can think of right now. Browser CSS parsers are all much more advanced and tested than most libraries. And when something is not understood by the browser CSS parser, the original particular styles would not be usable anyway by that browser.

Gutenberg frontend testing is already performed in a headless but otherwise complete browser (JS API-wise), so this should not be a problem.

So yes, this solution appears to be indeed the best one. I can make a PR following this approach instead.

zaguiini · 2023-03-20T13:28:18Z

@strarsis I don't quite understand the solution using css-what as that library is a selector parser so how exactly are we able to transform and walk through the declaration lists (CSS rules blocks) using it?

I haven't had the time to fully play with it, though, but it looks like it won't let the user add additional transformers.

strarsis · 2023-03-21T18:49:51Z

@zaguiini: The browser does the CSS parsing on rule level. There is no browser API yet for parsing the selectors themselves, hence CSSWhat is used. The CSS values can also be parsed using the browser API (CSSStyleValue), the browser support improved a lot since then (upcoming Safari should have it, Firefox does not, but this can change when all browsers finally support it). But CSS values can also be parsed using a JavaScript library. This is not 100% perfect, but lots of parsing is delegated to the browser. And the required additional bytes are much, much less than using a full CSS parser.

Concerning the extensibility: Well, for Gutenberg style isolation only two tasks are needed: 1. Wrap the selectors (prefix with a class selector), 2. Rewrite the url(...) references, so the style-relative URIs still work when those styles are inlined in editor.
Adding new tasks is also possible, as the browser handles the rule level CSS parsing, and with more time the browser APIs should become so good that it can basically replace a CSS parser library, but with the same compatibility and flexibility as the browser itself.

strarsis · 2023-03-21T19:29:20Z

@youknowriad, @kraftner, @zaguiini; @dsturm:
After some more experimentation I found an extremely easy and nearly 100% supported approach for style isolation:
https://codepen.io/strarsis/pen/zYJmpoV

This uses Shadow DOM (Web Components API) (no <iframe>s!) and can be even be directly used with React (on which Gutenberg is currently based on): ReactShadow

No style post-processing necessary, just creating the shadow DOM for the editor - in open mode it can even be still easily accessed from the document. Styles are simply isolated inside.
This is in fact so easy and simple that I wonder whether someone else already tried this and found a show stopper? But I cannot find one, and together with React-compatibility this appears to be the ideal solution to the style isolation problem.

youknowriad · 2023-03-22T08:42:27Z

@strarsis I know @ellatrix already tried shadow DOM at multiple occasions. Not sure how far we went there.

strarsis · 2023-03-22T18:13:32Z

@zaguiini, @youknowriad: The Lightning CSS parser is written in Rust, but can be used in browser (WASM (Web Assembly)). It has a size of 20 kB (minified) / 5.4 kB (minified+gzipped):
https://bundlephobia.com/package/lightningcss-wasm@1.19.0
The Servo browser uses this, so it should be quite feature-complete.
Using this avoid the necessity of CSSWhat and other helper libraries that have to polyfill or complement the CSS Typed OM API

zaguiini · 2023-03-22T18:15:36Z

@strarsis Lightning CSS is not this small 😅 Been there, done that. From above:

I tried using lightningcss but you need to do that top-level await madness in order to kick off the WASM download (which, btw, is 8 MB).

strarsis · 2023-03-30T14:35:12Z

@youknowriad, @kraftner, @zaguiini; @dsturm:
PR for post-processing styles using the browser APIs (with supplemental libraries):
#49483

strarsis · 2023-10-20T20:41:40Z

New editor styles transformation approach using PostCSS is now merged (many thanks @zaguiini)!

I am closing this PR in favor of the now merged in PostCSS method, which has a much smaller footprint and excellent style manipulation capabilities.

Use css-tree for block-editor style transformations.

ea8665f

Use WHATWG URL for block-editor style URL manipulations.

strarsis requested review from ajitbohra, ellatrix, gziolo, nerrad, ntwb and youknowriad as code owners September 21, 2020 17:41

strarsis mentioned this pull request Sep 21, 2020

Use CSSTree for style transformations (new) #21936

Closed

5 tasks

Revert puppeteer config from WSL2 friendly variant.

0893926

strarsis changed the title ~~Use css-tree for block-editor style transformations.~~ Use css-tree for block-editor style transformations (3). Sep 21, 2020

youknowriad reviewed Sep 22, 2020

View reviewed changes

strarsis added 3 commits September 22, 2020 12:16

Remove unused ltrim package.

edf6c8a

Use normal function type for trimOutmostChars.

Cosmetic code change.

cd093c6

Use dist build of css-tree (browser compatibility / linter).

b4b7bc2

youknowriad reviewed Sep 22, 2020

View reviewed changes

packages/block-editor/package.json Outdated Show resolved Hide resolved

youknowriad reviewed Sep 22, 2020

View reviewed changes

packages/block-editor/src/utils/transform-styles/test/traverse.js Show resolved Hide resolved

youknowriad reviewed Sep 22, 2020

View reviewed changes

packages/block-editor/src/utils/transform-styles/transforms/wrap.js Show resolved Hide resolved

strarsis added 2 commits September 22, 2020 14:10

Replace other occurences of css-tree requires with css-tree dist requ…

62f2fa4

…ires.

Remove useless empty line at beginning of file.

911c4d8

Use csstree without dist (to reduce bundle size).

10d9853

Base automatically changed from master to trunk March 1, 2021 15:44

strarsis mentioned this pull request Aug 18, 2021

Error while traversing the CSS: property missing ':' #19930

Closed

kraftner mentioned this pull request Apr 19, 2022

Fix or replace CSS Parser that transforms editor styles #40444

Closed

zaguiini reviewed Mar 13, 2023

View reviewed changes

strarsis mentioned this pull request Mar 30, 2023

Use browser parser APIs for block-editor style transformations. #49483

Closed

strarsis closed this Oct 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use css-tree for block-editor style transformations (3). #25514

Use css-tree for block-editor style transformations (3). #25514

strarsis commented Sep 21, 2020 •

edited

strarsis commented Sep 21, 2020

youknowriad left a comment

youknowriad Sep 22, 2020

strarsis Sep 22, 2020 •

edited

strarsis commented Sep 22, 2020 •

edited

youknowriad commented Sep 22, 2020

strarsis commented Sep 22, 2020 •

edited

strarsis commented Sep 22, 2020

youknowriad commented Sep 23, 2020

strarsis commented Aug 18, 2021 •

edited

strarsis commented Feb 22, 2022

zaguiini commented Mar 13, 2023 •

edited

zaguiini Mar 13, 2023 •

edited

strarsis Mar 13, 2023

strarsis commented Mar 13, 2023 •

edited

strarsis commented Mar 14, 2023

zaguiini commented Mar 17, 2023 •

edited

youknowriad commented Mar 20, 2023

kraftner commented Mar 20, 2023

strarsis commented Mar 20, 2023 •

edited

kraftner commented Mar 20, 2023

strarsis commented Mar 20, 2023 •

edited

youknowriad commented Mar 20, 2023

strarsis commented Mar 20, 2023 •

edited

zaguiini commented Mar 20, 2023 •

edited

strarsis commented Mar 21, 2023

strarsis commented Mar 21, 2023

youknowriad commented Mar 22, 2023

strarsis commented Mar 22, 2023

zaguiini commented Mar 22, 2023

strarsis commented Mar 30, 2023

strarsis commented Oct 20, 2023 •

edited

Use css-tree for block-editor style transformations (3). #25514

Use css-tree for block-editor style transformations (3). #25514

Conversation

strarsis commented Sep 21, 2020 • edited

Description

How has this been tested?

Types of changes

Checklist:

strarsis commented Sep 21, 2020

youknowriad left a comment

Choose a reason for hiding this comment

youknowriad Sep 22, 2020

Choose a reason for hiding this comment

strarsis Sep 22, 2020 • edited

Choose a reason for hiding this comment

strarsis commented Sep 22, 2020 • edited

youknowriad commented Sep 22, 2020

strarsis commented Sep 22, 2020 • edited

strarsis commented Sep 22, 2020

youknowriad commented Sep 23, 2020

strarsis commented Aug 18, 2021 • edited

strarsis commented Feb 22, 2022

zaguiini commented Mar 13, 2023 • edited

zaguiini Mar 13, 2023 • edited

Choose a reason for hiding this comment

strarsis Mar 13, 2023

Choose a reason for hiding this comment

strarsis commented Mar 13, 2023 • edited

strarsis commented Mar 14, 2023

zaguiini commented Mar 17, 2023 • edited

youknowriad commented Mar 20, 2023

kraftner commented Mar 20, 2023

strarsis commented Mar 20, 2023 • edited

kraftner commented Mar 20, 2023

strarsis commented Mar 20, 2023 • edited

youknowriad commented Mar 20, 2023

strarsis commented Mar 20, 2023 • edited

zaguiini commented Mar 20, 2023 • edited

strarsis commented Mar 21, 2023

strarsis commented Mar 21, 2023

youknowriad commented Mar 22, 2023

strarsis commented Mar 22, 2023

zaguiini commented Mar 22, 2023

strarsis commented Mar 30, 2023

strarsis commented Oct 20, 2023 • edited

strarsis commented Sep 21, 2020 •

edited

strarsis Sep 22, 2020 •

edited

strarsis commented Sep 22, 2020 •

edited

strarsis commented Sep 22, 2020 •

edited

strarsis commented Aug 18, 2021 •

edited

zaguiini commented Mar 13, 2023 •

edited

zaguiini Mar 13, 2023 •

edited

strarsis commented Mar 13, 2023 •

edited

zaguiini commented Mar 17, 2023 •

edited

strarsis commented Mar 20, 2023 •

edited

strarsis commented Mar 20, 2023 •

edited

strarsis commented Mar 20, 2023 •

edited

zaguiini commented Mar 20, 2023 •

edited

strarsis commented Oct 20, 2023 •

edited