[all components] Use ESM metadata constants#5248
Conversation
commit: |
Bundle size
PerformanceTotal duration: 1,210.67 ms -62.93 ms(-4.9%) | Renders: 78 (+0) | Paint: 1,881.68 ms -87.95 ms(-4.5%)
11 tests within noise — details Metric alarms
Check out the code infra dashboard for more information about this PR. |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
d34678f to
5ff6d87
Compare
|
Checking the diff. Here's my human review: Imo there are several downsides to this setup
In short, if code infra has to adopt this complexity, I first want to make sure it's the right solution to the problem. The thing is that our users already have tools in their toolchain that can inline values like this. e.g. terser will take the following const MyEnum = {
f: 'foo'
}
export const y = {
[MyEnum.f]: 'bar'
}and happily produce export const y={foo:"bar"};out of it in your app bundles if you use it. The problem we have and the reason why this is not already automatically happening is that babel typescript emits those enums as let MyEnum = function (MyEnum) {
MyEnum["f"] = "foo";
return MyEnum;
}({});
export const y = {
[MyEnum.f]: 'bar'
}and terser doesn't know how to inline this. It will produce export const y={[function(y){return y.f="foo",y}({}).f]:"bar"};Moreover, different compilers emit different things for these enums, and exactly because they hold runtime semantics based on types, typescript suggests to keep away from them ( The only real downsides I see
edit: Just noticed that the pattern with |
|
With this, I can see scenarios were the end-users final bundle has more size than before where the code could be - import a from './a.js';
//...
style={{[a.b]: '43px', [a.c]: '100px'}}but is now - style={{['--accordion-panel-height']: '43px', ['--accordion-panel-width']: '100px'}} |
👍 and this supports exactly why this is best left as a concern for the minifier stage on the end-user build pipeline. The minifier can measure and decide whether it will be an overall optimization or not, based on the usage across the full end-user bundle. |
|
@Janpot @brijeshb42 thanks for checking here. I don't think leaving the consumer to optimize is realistic, most will not do it or know how to do it.
I can't measure an actual huge realistic app, but I checked empirically by measuring a Vite app that includes React and uses 10 Base UI components. All numbers below are gzip deltas (each variant was built as a published package and then bundled through Vite)
"Named ESM string export" means defining each value as an ordinary JavaScript binding: export const checked = 'data-checked';
export const unchecked = 'data-unchecked';This is similar to the |
Yes, I think I'd like to frame this more as "optimizing for statically analyzable code, which underlying tooling will be able to pick up on more and more over time is a more durable long term solution". Or "unless there is a significant win, it may be more durable to leave this an end-user tooling concern and optimize for machine-readability".
Yes, absolutely. Interesting differences in output for
Surprised to find it also perfectly inlines certain patterns. I don't see a reason why it couldn't inline the A few notes:
edit: just realized vite isn't built on esbuild anymore, but ok... edit 2: Interestingly, |
|
Prototyped a version of our build tool that resolves internal |
|
I tried with named ESM string exports & the Rolldown build resolves the namespace re-exports as intended. Compared with full enums using normal Babel output in the same realistic 10-comp Vite app:
So it works. But a blocker is docs generation, the current pipeline only extracts |
8e063c3 to
86a9f4a
Compare
Base UI (mui/base-ui#5248) is moving data-attribute names from TS string enums to named ESM exports -- `export const checked = 'data-checked'` read as `import * as FooDataAttributes from './metadata'` / `FooDataAttributes.checked` -- so the value is authored once but referenced everywhere. Under preserveModules rolldown keeps the cross-module import (it does not inline constants), which ties every referencing module to the metadata module and defeats consumer tree-shaking. Base UI previously solved this with a checker-backed Babel transform in its own repo and removed it; this moves the equivalent into the shared build. Add a Babel plugin, run in the rolldown transform, that replaces cross-module `data-*` string-constant references with their literal. A pre-scan collects every `export const NAME = 'data-...'` up front (order-independent), and the plugin resolves each relative import against it. Running at transform time means Babel's scope resolution decides what is the imported binding, so a shadowing `function f(open) {}` is left alone; rolldown then drops the now-dead import, and the metadata module falls out of any consumer that only used data attributes. Handles `import * as ns` member access (`ns.open`, `ns['open']`) and named imports, removing specifiers/imports that become fully consumed and keeping those still needed for non-data members. A reference that cannot be resolved or matched is left untouched -- a missed constant is a smaller optimization, never a miscompile, so nothing here is fatal. Verified on a Base-UI-shaped fixture end to end: a consumer using only a data-attribute pulls the literal and nothing from the metadata module. Real packages declare no such constants, so they build unchanged (0 inlined).
Base UI declares CSS variables the same way it declares data attributes -- `export const popupWidth = '--popup-width'` in a `*CssVars` module, read through a namespace import -- and they have the same problem: the cross-module reference keeps every consumer tied to the constants module. Widen the match from `data-*` to also cover `--*`, and rename the plugin to metadata constants, the term Base UI uses for both (mui/base-ui#5248). A bare `--` is excluded: that is the end-of-options marker, not a custom property, so at least one character after the prefix is required. Inlining stays safe for the same reason as before -- these are immutable primitives, so duplicating them at call sites cannot change behaviour.
|
It's only a few bytes but still interesting how the inlined variant wins compressed. Did some benchmarking of the different solutions myself, and it seems to roughly reproduce. 2 benchmarks, a 10 BUI component page, and a page with every component. Built with vite+rolldown-vite, gzip size and brotli size. Because anyone who cares about shaving off <1kb on a 100+ kb bundle would have already set up their env with best in class minifier and best in class compression.
We see on the small app the rolldown variants are roughly neutral. the babel-built consts and the checker inliner take small gz/br wins. Conclusions:
|
Updating in mui/mui-public#1713 |
Replace the
DataAttributesandCssVarsstring enums with named ESM exports so application bundlers can inline and tree-shake the values without a custom TypeScript-checker transform.Changes
@mui/internal-code-infra@988e42d.Bundle size
Measured in the same realistic Vite consumer using React and ten Base UI component families. Deltas are relative to the full-enum source built through normal Babel output.
The full built-package namespace measures 447,379 B parsed / 145,764 B gzip.
Validation
pnpm typescriptpnpm eslintpnpm test:jsdom --no-watch— 6,817 passing testspnpm --filter @base-ui/react test:packageRemaining before merge
@mui/internal-docs-infracurrently extracts data attributes and CSS variables only from enum declarations. It needs to support these named metadata exports without adding runtime compatibility code.@mui/internal-code-infradependency after [code-infra] Build with rolldown: resolve imports + flatten re-exports mui-public#1674 is merged and published.