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

Style-Engine: Make the Style Engine Store hookable #42493

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

aristath
Copy link
Member

@aristath aristath commented Jul 18, 2022

What?

Adds a new get_stores() method to get an array of all available stores, and a wp_style_engine_css_rules_store filter to allow filtering/mutating the store.

Why?

The style-engine should be hookable and allow customizations. The style engine is not just something that Core will use, it's something that can benefit plugins & themes as well.
By adding a hook in the store, we can allow caching plugins to do what they need to do in order to improve performance. Depending on the size of the inlined styles, one can choose to compile them to a file for example instead of have them rendered inline. Themes will be able to override CSS directly in the style engine instead of adding extra stylesheets.
One of the core philosophies of WP is that the system should be extensible and hookable, so this PR ensures that the style-engine is not a closed box.

@aristath aristath requested review from ramonjd and andrewserong July 18, 2022 08:07
@ramonjd
Copy link
Member

ramonjd commented Jul 18, 2022

The style-engine should be hookable and allow customizations. The style engine is not just something that Core will use, it's something that can benefit plugins & themes as well.

Agree 100%. And thanks for throwing up an example of where a WP hook would make sense. 🙇

This is the first time the style engine will be in core, and it status will continue to be experimental, so I’d be wary of exposing internals introducing any more functionality before then personally, at least until we’ve stabilized the APIs and definitely after we've done the performance test work.

Am I being too paranoid?

I'm just thinking that the core phase 1 goals are not yet delivered so it might be too early to commit to hooks until things are working.

Furthermore, come 6.1 there won't be much to hook into. Layout and probably elements links only. Once we have global styles migrated and few more areas, it'll be meatier and infinitely more testable.

Sorry to always sound like a handbrake!

I'm not against adding hooks at all in fact, what you've described is going to be a powerful feature for theme authors, I'm just wondering if the use case makes sense given the infancy of the library.

Aside for all that, these sorts of PRs are still very useful and it's never too early to start thinking about where hooks would make sense in general.

@andrewserong
Copy link
Contributor

Agree 100%. And thanks for throwing up an example of where a WP hook would make sense.

I absolutely agree with the long-term vision for this.

I’d be wary of exposing internals introducing any more functionality before then personally, at least until we’ve stabilized the APIs and definitely after we've done the performance test work.

I also agree with this. From my perspective, adding a hook and opening up the style engine for extensibility sounds like the very final step we should take. With the original goal of the project being to consolidate internal style generation for block supports and global styles, I prefer the idea of taking the iterative approach of treating it as an internal API, and once it's feature-complete and stable, then exposing it for extensibility.

It's a great exploration to have, though, to guide our implementation to ensure that we can neatly expose it in a PR like this one once we're ready and the API has stabilised.

Aside for all that, these sorts of PRs are still very useful and it's never too early to start thinking about where hooks would make sense in general.

A big +1 to this — it's also a great place to capture the discussion, particularly since it's likely to be something that lots of other folks will think of / propose, too.

@aristath
Copy link
Member Author

Thank you both for the feedback! Makes perfect sense to put this on hold until after 6.1, to get the API in a solid state and revisit at a later date.
I'll just leave it open for now so we have it on our radar 👍

@flexseth
Copy link
Contributor

flexseth commented Sep 5, 2023

@aristath - I mentioned this comment in a use case regarding theme overrides. Namely, adding Drop Caps back into TwentyTwentyThree (and other themes). Feel like theme authors should be able to modularly and programmatically override these options. Or build plugins to provide overrides for the community to use!

It is possible that this is made possible in JavaScript by accessing the datastore that the styles exist in, that's a little bit over my head but it seemed like a perfect php filter hook for Core.

+1 for the idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Style Engine /packages/style-engine
Projects
Status: Blocked
Development

Successfully merging this pull request may close these issues.

4 participants