-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Comments
@WordPress/gutenberg-components I'd love to get your take on this one. Essentially we are running into many issues working on the |
The gutenberg/packages/components/src/index.ts Line 202 in e2142c3
|
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. |
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 :) |
@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 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 |
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. |
@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 :) |
@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 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 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 |
@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. |
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... |
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:
Then, we can support both use-cases in a standard way. Win win? |
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 This is obviously an issue that is common in the JS community and not really a Gutenberg or a WordPress issue. 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? |
@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. ( 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 :) |
Folks, I think the solution we were looking for just fell from the sky and into our hands :) https://react.dev/reference/react-dom/components/style#special-rendering-behavior I now think the best plan of action is wait until this is officially supported and we are in a version of React that supports it, and then just use inline styles handled and de-duplicated by React. Basically, CSS-in-JS but without any libraries, future-proof, minmal bundle size and maximal performance. I'm not super-familiar with the inside/outside iframe styles situation, but if Emotion worked then I'm fairly confident this approach will also work, since it fundamentally does the same (inject some de-duplicated inline styles). Thoughts? |
In theory that sounds like it could solve it. However, I'm a bit concerned about how it would handle the Imagine loading component X and component Y, and their styles come with conflicting precedence - they're using the same selector and providing a different value for a particular style property. There is a chance that the value will depend on the load order of the components - if X renders before Y, Y's styles will be used, and the other way around. Perhaps React has found a way to always have the same stylesheet order, but that sounds like an impossibly tricky problem to solve, especially as you move styles around, create new stylesheets, and remove old ones. FWIW currently in Calypso, we have similar issues with lazy-loaded components when precedence is not well sorted out. |
Plus style precedence should be handled through CSS Cascade Layers nowadays, a native CSS feature supported in all browsers for that exact use-case. In the components package, specifically, I already have a draft proposal that will suggest using them to fix these issues you're referring to in a clean manner. |
Also, to be clear, replacing emotion with this React API (and something like |
Another relevant note is that there's already some alignment around using layers for other matters within WordPress: #61810 |
We have style overrides |
@DaniGuardiola I guess that assumes rewriting all existing styles to use cascade layers? If that's what you mean, won't that cause compatibility issues with existing plugins and themes that rely on overriding these styles the old way? |
What do you mean by "the old way"? Unlayered styles always have priority over layered styles, so unless I'm missing something those overrides should still work. |
Most styles across plugins and themes will be unlayered, so yes, that's what I mean by the old way. What you're saying makes sense, although I'm not 100% convinced all custom 3rd party CSS will work the same way. Also, a migration could be interesting since it may cause some styles to be layered, and some not, and that might lead to unexpected precedence issues. |
@ellatrix can you please making the hook public? I am trying to replicate the Gutenberg gallery block to use it as a starting point for my custom block but in doubts how to going further as looks using private apis is forbidden in plugins. |
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
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 havereact
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 :)The text was updated successfully, but these errors were encountered: