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

Expose @emotion/styled to extenders #61232

Open
fabiankaegy opened this issue Apr 30, 2024 · 13 comments
Open

Expose @emotion/styled to extenders #61232

fabiankaegy opened this issue Apr 30, 2024 · 13 comments
Labels
Developer Experience Ideas about improving block and theme developer experience [Package] Components /packages/components [Type] Enhancement A suggestion for improvement.

Comments

@fabiankaegy
Copy link
Member

Several of the ´@wordpress/´ packages use @emotion/styled internally as their CSS in JS solution.

When 3rd party block component packages then try to also use the same library first of all it causes a lot of duplicate code to be loaded slowing down the editor. But on top of that emotion also adds the following warning in the browser console:

Warning

You are loading @emotion/react when it is already loaded. Running multiple instances may cause problems. This can happen if multiple versions are used, or if multiple builds of the same version are used.`

Proposed solution

It would be geat to either be able to import the functions used by the components package like useCx styled etc. in external packages.

Alternatively maybe @emotion/styled & @emotion/react could get bundled as their own packages in WordPress core similar to how we have react bundled in core.

In the end I think the ideal solution would be to not directly expose emotion or any other CSS in JS library but instead expose the abstracted layer that allows anyone to use the library without having to worry about whether a component gets rendered inside an iframe or not etc. Just making it easy to use the same component styling system that core already is using internally :)

@fabiankaegy fabiankaegy added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components Developer Experience Ideas about improving block and theme developer experience labels Apr 30, 2024
@fabiankaegy
Copy link
Member Author

@WordPress/gutenberg-components I'd love to get your take on this one.

Essentially we are running into many issues working on the @10up/block-components package with having to duplicate the efforts that are already handled by core here and would love to simplify that setup to reduce the bloat that needs to get introduced.

@fabiankaegy
Copy link
Member Author

The __experimentalStyleProvider is already getting exported here:

export { default as __experimentalStyleProvider } from './style-provider';
but without the other pieces of emotion it sadly isn't very helpful

@mirka
Copy link
Member

mirka commented May 1, 2024

I'm afraid we're planning on moving off of Emotion, and more generally any kind of CSS-in-JS library that does any runtime work. The three main motivators being that we've been finding it ultimately unnecessary, we're starting to see actual performance costs, and the bundle size cost that it adds.

I'm curious though, does this affect an extender's decision to adopt Emotion in their projects? Even though we intentionally weren't exposing Emotion functions because we didn't want to commit to endorsing it publicly, was the fact that wp-components was using Emotion under the hood encouraging extenders to use it too?

I also would like to know if you've been encountering use cases where Emotion is necessary, rather than simply being convenient. Or, which parts of Emotion do you find the most value in.

@fabiankaegy
Copy link
Member Author

Thanks for those insights @mirka :)

To give a bit of insight, we would also in theory find a way to get rid of CSS in JS. But the thing that is keeping us there is that the way we distribute our component library is as an NPM package. And so far we've not found a way to easily ship a stylesheet alongside the library without adding a lot of additional effort to the consumers or the library to manually enqueue the CSS on all the editor screens and in all the framed and not iframed editors.

Since we've not found a solution for that we needed to look at CSS in JS solutions and then started adopting emotion because core also used it. Ideally we want all our components to he built in a way where they could be brought over to core if we find that there is a use-case for it :)

@DaniGuardiola
Copy link
Contributor

@fabiankaegy I'm not yet entirely familiar with the full context of this problem space, but I've observed that many UI libraries have standardized on using CSS imports which, while not "spec" standard, are pretty standard indeed in practice and a basic feature of any bundler out there (they will also be closer to "spec" standard when import attributes land). It's also definitely the simplest approach.

Even if we want to support a bundle-less solution, we could engineer a very simple "singleton" insertion mechanism as a fallback, and with CSS Cascade layers (which I'm gonna push for) style precedence won't even be an issue then. Or, as a last resort, we could just shift the responsibility of importing styles to the user which is typically trivial.

A full CSS-in-JS approach is not necessary in any case, and I feel like it'd be trivial to implement a "singleton" approach as I described above with useInsertionEffect or a more vanilla approach.

Of course, I'm just thinking out loud here and none of this is normative, but this is one of the things I plan to focus on in the near future.

Tangentially, the overall trend I'm seeing is towards less wrappers and re-exports, e.g. we're now encouraging direct imports of React rather than the @wordpress/elements package: #54074 (thanks @mirka for sharing it in another thread)

@fabiankaegy
Copy link
Member Author

Hey @DaniGuardiola 👋 Thanks also for your insights :)

I am completely on board with everything you are saying. But I'm still not sure how a component library would be able to actually include some CSS that gets enqueued properly inside and outside the iframed editor without creating a bunch of work for the user of the component library.

That is the main hurdle which keeps us using a CSS in JS solution. Because we can handle the logic for where the stylesheet gets injected for the consumer of the library.

@DaniGuardiola
Copy link
Contributor

DaniGuardiola commented May 6, 2024

@fabiankaegy what do you mean by "where the stylesheet gets injected"? Are you referring to the order relative to other stylesheets that might or might not override the styles? Or just about the process of loading them in the page, regardless of order?

Also, I'm not sure I'd call using any bundler that supports CSS imports (all of them pretty much) "a bunch of work". It's a pretty standard / out-of-the-box thing these days and almost required for a sane bundle size (minification also observably speeds up parsing and execution of JavaScript). This also only affects those consumers that use the components package specifically, which is a smaller subset, so I feel like requiring it is reasonable.

Of course, there's the matter of backwards compat, which might be the thing that renders this approach ultimately not possible, but I think it's worth considering at least, as well as potential fallbacks as I shared earlier :)

@fabiankaegy
Copy link
Member Author

@DaniGuardiola What I'm talking about is how the styles actually get enqueued in WordPress.

If you want to load some JS in any of the editors today you need to enqueue it via wp_enqueue_script on the enqueue_block_editor_assets hook. This takes care of loading that JS in all the editors in WordPress. So today when someone uses any of our react components in the editor all they have to do is import the component into that JS file and they are good to go.

If we were to not use a CSS in JS solution the consumer of our library would now also need to manually import the CSS somewhere and enqueue it on both the enqueue_block_editor_assets and enqueue_block_assets hook with a conditional to only load it if is_admin is true. The reason for this is that the JS may portal into the editor canvas iframe but also place elements outside the iframe. So the CSS needs to be loaded in both contexts.

As far as I'm aware there is no simple way to do all of this as the publisher of an NPM package on behalf of the user if there isn't a runtime level thing happening to inject the styles wherever the component is used.

In a webpack context having a import "@10up/block-components/dist/style.css" would only result in the CSS getting output in an additional CSS file which I now manually need to worry about

@mirka
Copy link
Member

mirka commented May 6, 2024

@youknowriad @ellatrix Any way we can make this easier for third-party component library consumers without having them rely on runtime CSS-in-JS solutions? From the current state of dev ex it does indeed sound like CSS-in-JS is the most convenient here, which is unfortunate for overall performance.

@fabiankaegy
Copy link
Member Author

The one alternative I have considered quite a few times is instead of distributing the component library via NPM, it could also get distributed as a WordPress plugin. And therefore handle all the enqueueing manually same as WordPress core does for all the bundled packages.

Consumers of these library would then need to add the library as a script dependency in WP and that should take care of everything.

But shipping it as a plugin comes with quite a lot of additional overhead and other problems...

@DaniGuardiola
Copy link
Contributor

Thinking outside the box a bit here but, what if we "added support for CSS imports" in those hooks in some way? Seems like some very basic regex matching (in lack of actual AST parsing) could be good enough to pull this off:

  1. Find CSS imports in JavaScript assets being loaded.
  2. Remove from JS before loading.
  3. Load corresponding stylesheets as assets.

Then, we can support both use-cases in a standard way. Win win?

@youknowriad
Copy link
Contributor

I'm not entirely sure I understand all the nuance of all the comments here but here's what I'm understanding:

The main issue @fabiankaegy is raising is the fact that npm packages have a hard time exposing CSS and JS at the same time while allowing a good DevX import MyComponent from 'my-library and get everything working by default.

This is obviously an issue that is common in the JS community and not really a Gutenberg or a WordPress issue.
In fact, as you can see in our "Gutenberg As A Framework" website https://wordpress.org/gutenberg-framework/docs/intro, it's an issue for npm packages shipped by Core as well. Third-parties need to import these CSS files manually.

When it comes to npm packages, my opinion is that we should not introduce something in the output that is not standard yet. Sure CSS imports are widely used but I believe we should stick with standards and this is why Core packages ship one or multiple CSS files at the moment (or use some CSS in JS).

Now, for importing CSS into the WordPress block editor by third-party developers, I do believe we need to offer a way to mark some CSS as "content" which means the output gets loaded into the iframe/previews... and some other CSS as regular stylesheet (UI editor CSS). I believe @ellatrix has worked on this before, I'm pretty sure there are ways to do that already. Maybe we're lacking some documentation?

@fabiankaegy
Copy link
Member Author

@youknowriad I agree that this issue exists in any JS application. However for WordPress builds it is a little more complex still because we have to load these styles both in the iframe and outside the iframe.

Most of the time developing custom blocks you don't really need to be aware of the whole iframe stuff. That is a lower level thing that just works. So it is asking quite a bit of a library consumer to ensure that the library styles are manually enqueued everywhere.

Yes there are APIs to load styles inside and outside of the iframe. (enqueue_block_editor_assets / enqueue_block_assets). But when someone is just developing a block where all they do is adding their editorScript via block.json, the last thing you want to do is require them to do is manually add some PHP in the main plugin / theme code to enqueue some library CSS in a few different contexts.

Because of all this complexity we currently are pretty locked into using a CSS in JS solution. But because core is doing the same thing without exposing it to be used by others we have to load it multiple times with possibly different versions which causes all kinds of issues and bloat.

I totally get that core doesn't want to get locked in to some API like this. I'm just pointing out what issues we are facing and what few alternatives we currently have :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Experience Ideas about improving block and theme developer experience [Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

4 participants