wp-calypso icon indicating copy to clipboard operation
wp-calypso copied to clipboard

UI: Try split component entries

Open mirka opened this issue 6 months ago • 5 comments

Related to ARC-199

Proposed Changes

This is the latest take on CSS splitting ("level 2" as in ARC-170) in @automattic/ui. Stylesheets are prebuilt, but not bundled together in a single root file.

TODO

  • [ ] Update the readme if we're going with this

Why are these changes being made?

To balance CSS bloat concerns with developer ergonomics.

The caveats for the consumer being:

  • The consumer must import from a subdirectory (import { Badge } from "@automattic/ui/badge").
  • The consumer's build tooling must be able to handle the CSS loading.

To me this is an acceptable balance of concerns.

  • As maintainers, the exports are managed in a DRY and non-hacky way (exports field in package.json). The root barrel file is completely gone,
  • The only weird part is where we inject the import './index.css' statement in each JS bundle. I bet we could hook into it in a more cleaner way, but I doubt it would be simpler.
  • As consumers, the added inconvenience of a subdirectory import is not too bad.

Testing Instructions

yarn workspace @automattic/ui build

I also tested that it works in a default Vite project.

Pre-merge Checklist

  • [ ] Has the general commit checklist been followed? (PCYsg-hS-p2)
  • [ ] Have you written new tests for your changes?
  • [ ] Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • [ ] Have you checked for TypeScript, React or other console errors?
  • [ ] Have you tested accessibility for your changes? Ensure the feature remains usable with various user agents (e.g., browsers), interfaces (e.g., keyboard navigation), and assistive technologies (e.g., screen readers) (PCYsg-S3g-p2).
  • [ ] Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • [ ] Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
    • [ ] For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • [ ] For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

mirka avatar Jun 16 '25 10:06 mirka

Link to live branch is being generated... Please wait a few minutes and refresh this page.

github-actions[bot] avatar Jun 16 '25 10:06 github-actions[bot]

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • notifications
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug try-split-components on your sandbox.

matticbot avatar Jun 16 '25 10:06 matticbot

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

matticbot avatar Jun 16 '25 10:06 matticbot

@Automattic/team-calypso Worth trying or not? I don't feel strongly either way, but this nascent low-stakes package may be a good place to test out the strategy. If we don't think it's worth it, we can continue shipping with the single stylesheet model.

mirka avatar Jun 17 '25 18:06 mirka

but I'm not as convinced to be doing it here and now, and particularly in the Calypso work (vs. admin). Or is there an urgent need for this from consumers?

I don't think there's any justifiable need as of now, given that the package is tiny. The only reason to do it now is that, if it's ever going to happen, we might as well start now so people don't have to switch later. Though, even switching later is not going to be much of an issue since it's probably trivial to code mod.

mirka avatar Jun 18 '25 15:06 mirka

I compared the versions in #104243 and #104334 and noticed that the common non-split entrypoint adds some complications. For each sub-component, the styles are duplicated in the component-specific style (./badge/index.css) and the common style (./index.css). If the app mixes the two kinds of imports, the styles will always be duplicated.

The common entrypoint also creates multiple shared chunks with hashed names. The bundler can avoid duplicating JS code, but can't avoid duplicating CSS.

To do both common and split imports without any tradeoffs, we probably need less bundling. We don't really need tsup to bundle multiple JS files together. They can be copied to the dist folder one-to-one and continue to import each other. The bundler's only value-adding job is compiling the CSS modules. Mangle the class names and make the names part of the JS code. And create a .css file that doesn't need any further processing: it can be sent to the browser as it is.

jsnajdr avatar Jun 20 '25 08:06 jsnajdr

To do both common and split imports without any tradeoffs, we probably need less bundling.

If we want to go that route, then we may simply use tsc but it won't handle types for CJS bundles very well.

manzoorwanijk avatar Jun 20 '25 08:06 manzoorwanijk

This PR has been marked as stale. This happened because:

  • It has been inactive for the past 3 months.
  • It hasn't been labeled `[Pri] BLOCKER`, `[Pri] High`, `[Status] Keep Open`, etc.

If this PR is still useful, please do a trunk merge or rebase and otherwise make sure it's up to date and has clear testing instructions. You may also want to ping possible reviewers in case they've forgotten about it. Please close this PR if you think it's not valid anymore — if you do, please add a brief explanation.

If the PR is not updated (or at least commented on) in another month, it will be automatically closed.

github-actions[bot] avatar Sep 20 '25 00:09 github-actions[bot]

This PR has been automatically closed as it has not been updated in some time. If you want to resume work on the PR, feel free to restore the branch and reopen the PR.

github-actions[bot] avatar Oct 22 '25 06:10 github-actions[bot]