UI: Try split component entries
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 (
exportsfield 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)?
Link to live branch is being generated... Please wait a few minutes and refresh this page.
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.
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.
@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.
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.
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.
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.
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.
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.