[all components] Reduce shared bundle size

[atomiks]

Summary

Reduces shared bundle cost across the popup, focus-management, and selector paths used by many Base UI components.

This keeps the measurable wins from the internal tabbable helper and the plain-vs-memoized selector split, trims shared utility code used across popup-heavy components, and backs out the unverified @base-ui/utils/store/... import-path churn so store consumers continue to use the standard barrel entrypoint.

Changes

• Replace the external tabbable dependency with a smaller internal helper and add focused regression coverage for shadow DOM, details, hidden controls, radio groups, and inert content.
• Split memoized selector logic so entrypoints that only need plain selectors no longer pull memoized selector overhead.
• Simplify shared render, transition, animation, popup, and store utilities used across multiple components.
• Keep Store, ReactStore, and useStore imports on @base-ui/utils/store after the subpath split did not show a verified bundle benefit.

Notes

• The latest published bundle bot result on this PR reports @base-ui/react down by about 3.4 KB gzip versus master.
• Stripped production contained-trigger and detached-trigger benchmarks were effectively flat versus master.
• A small combobox-only follow-up also removes redundant non-reactive store selector subscriptions.

[mui-bot]

Bundle size report

Bundle	Parsed size	Gzip size
@base-ui/react	▼-4.41KB(-0.96%)	▼-1.6KB(-1.10%)

Details of bundle changes

---

Check out the code infra dashboard for more information about this PR.

[netlify]

✅ Deploy Preview for base-ui ready!

Name	Link
🔨 Latest commit	b6164d4
🔍 Latest deploy log	https://app.netlify.com/projects/base-ui/deploys/69c311e9c4cfa800088a8f5e
😎 Deploy Preview	https://deploy-preview-4336--base-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[pkg-pr-new]

• base-ui-tanstack-start

• vite-css-base-ui-example

  pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/react@4336
  

  pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/utils@4336
  

commit: b6164d4

[michaldudak]

Code review

Found 3 issues (scored 75/100 confidence -- below normal posting threshold, posting by request):

1. New test file uses Chai-style assertions (.to.equal()) instead of vitest-native matchers (.toBe()). Two tests are inconsistent with the rest of the file, which already uses vitest-native style. (AGENTS.md says "The repository is transitioning from chai and sinon, prefer vitest native functions for all new code.")

base-ui/packages/react/src/floating-ui-react/utils/tabbable.test.ts

Lines 56 to 58 in d2e76b4

	expect(isTabbable(element)).to.equal(true);
	expect(tabbable(document.body)).toContain(element);

base-ui/packages/react/src/floating-ui-react/utils/tabbable.test.ts

Lines 70 to 72 in d2e76b4

	expect(isTabbable(button)).to.equal(false);
	expect(tabbable(document.body)).not.toContain(button);

2. Toast store selector memoization uses module-level closure variables (lastToasts, lastMap), creating a single shared cache slot. If multiple ToastProvider instances coexist, they will thrash each other's cache on every render, returning new Map objects and breaking React memoization. The previous createSelectorMemoized used a per-instance WeakMap keyed by state.__cacheKey__. This is a performance regression, not a correctness bug.

base-ui/packages/react/src/toast/store.ts

Lines 34 to 66 in d2e76b4

	const toastMapSelector = (() => {
	let lastToasts: State['toasts'] | null = null;
	let lastMap: ToastMap = new Map();
	return (state: State) => {
	const { toasts } = state;
	if (toasts === lastToasts) {
	return lastMap;
	}
	const map: ToastMap = new Map();
	let visibleIndex = 0;
	let offsetY = 0;
	toasts.forEach((toast, toastIndex) => {
	const isEnding = toast.transitionStatus === 'ending';
	map.set(toast.id, {
	value: toast,
	domIndex: toastIndex,
	visibleIndex: isEnding ? -1 : visibleIndex,
	offsetY,
	});
	offsetY += toast.height || 0;
	if (!isEnding) {
	visibleIndex += 1;
	}
	});
	lastToasts = toasts;
	lastMap = map;
	return map;

3. Removed // We need to mutably own 'merged' comments in mergeProps.ts and mergePropsN. The spread on those lines is critical -- without it, mutablyMergeInto would corrupt the shared EMPTY_PROPS constant. The comment protected against future maintainers removing the "redundant-looking" spread.

base-ui/packages/react/src/merge-props/mergeProps.ts

Lines 63 to 65 in d2e76b4

	export function mergeProps(a: any, b: any, c?: any, d?: any, e?: any) {
	let merged = { ...getInputProps(a, EMPTY_PROPS) };

base-ui/packages/react/src/merge-props/mergeProps.ts

Lines 103 to 105 in d2e76b4

	let merged = { ...getInputProps(props[0], EMPTY_PROPS) };

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[LukasTy]

Claude Opus 4.6 review

Previously flagged issues — status check:

Issue	Status
Chai-style assertions (.to.equal()) in tabbable tests	Fixed — all tests now use vitest-native matchers (toBe, toContain, etc.)
Toast selector multi-provider isolation	Fixed — now uses WeakMap keyed by state.__cacheKey__ instead of module-level closure variables. Multiple providers each get their own cache entry
Removed safety comments in mergeProps about owning merged	Fixed — the comment // We need to mutably own \merged`.is now present above both [the spread inmergeProps](packages/react/src/merge-props/mergeProps.ts#L64) and [in mergePropsN`](packages/react/src/merge-props/mergeProps.ts#L102)
mergeProps first-getter mutation	NOT fully fixed — see Finding #1 below
useRenderElement ref-merging rerender coverage	Not addressed — no new rerender-driven tests added, but the structural change (always calling useMergedRefsN unconditionally) is actually safer than the old conditional hook pattern

---

New concerns found in this review:

1. mergeProps — getter-as-first-argument still mutates the returned object

In mergeProps.ts, the new mergeInto function:

function mergeInto(merged, inputProps) {
  if (typeof inputProps === 'function') {
    return inputProps(merged);  // no shallow copy
  }
  return mutablyMergeInto(merged, inputProps);
}

When the first argument is a getter, mergeInto({}, getter) calls getter({}) and returns the result directly. Subsequent mergeInto(merged, b) calls then mutate that result in place via mutablyMergeInto. The old createInitialMergedProps did { ...resolvePropsGetter(inputProps, EMPTY_PROPS) } — a shallow copy.

Practical risk: Low. I audited all call sites. No internal call passes a getter as the first argument to mergeProps. The only path where a getter could appear as first-in-array is via mergePropsN in useRenderElement, and those getters always return new objects. Still, it's a documented behavioral contract regression from the old implementation.

2. useAnimationsFinished — missing signal?.aborted guard on recursive exec()

In useAnimationsFinished.ts, the refactored catch block:

.catch(() => {
  if (treatAbortedAsFinished) {
    if (!signal?.aborted) { done(); }
    return;
  }
  // OLD: had `!signal?.aborted &&` before this check
  const currentAnimations = resolvedElement.getAnimations();
  if (currentAnimations.length > 0 && currentAnimations.some(...)) {
    exec(); // recurses even when signal is aborted
  }
});

The old code had !signal?.aborted && before the recursive exec() call. The new code drops this guard, meaning if treatAbortedAsFinished is false and the signal is aborted, exec() will still recurse uselessly (though done() will never fire since the .then() handler checks !signal?.aborted). Not a correctness bug, but wastes CPU and could loop unnecessarily.

3. tabbable — no tabIndex sorting (positive tabIndex elements not sorted before tabIndex=0)

The old tabbable library sorted positive-tabIndex elements before zero-tabIndex ones per browser spec behavior. The new implementation returns elements strictly in DOM order, regardless of tabIndex value.

Practical risk: Negligible. I searched the entire codebase and found no positive tabIndex usage — everything uses 0 or -1. However, missing a defensive test means a future regression could slip in.

4. useStableCallback — useRefWithInit usage with a ref argument

The refactored useStableCallback.ts passes callbackRef (from React.useRef) into useRefWithInit as an init argument. This is correct since useRefWithInit only runs the init function once, and the trampoline closes over callbackRef (a stable ref object). No behavioral regression — verified the initial-render-phase protection (assertNotCalled) still works.

5. useRenderElement — unconditional useMergedRefsN is an improvement

The old code had a rules-of-hooks violation: useMergedRefs / useMergedRefsN was called conditionally based on enabled, typeof document, and whether ref was an array. The new code always calls useMergedRefsN(refs) unconditionally with an empty array when disabled. This is structurally better and eliminates the rules-of-hooks concern.

6. useTransitionStatus — useMemo removal is safe

All callers immediately destructure the return value. Object identity doesn't matter since the individual values are primitives/stable setState refs.

7. Selector split — works correctly

createSelector is split to its own subpath (@base-ui/utils/store/createSelector) and the barrel @base-ui/utils/store still re-exports both createSelector and createSelectorMemoized. The exports field in package.json has the new "./store/createSelector" entry. Components that only need plain selectors (no memoization) can skip pulling in reselect. Toast store correctly does its own WeakMap-based memoization.

---

Summary

Finding	Severity	Blocking?
mergeProps getter-first mutation	Low	No (not hit internally)
useAnimationsFinished missing abort guard	Low	No (wastes CPU, no visible bug)
tabbable no tabIndex sorting	Low	No (not used)
Chai assertions fixed	-	Resolved
Toast multi-provider fixed	-	Resolved
mergeProps safety comments restored	-	Resolved

Overall assessment: The previously flagged issues are addressed. No blocking regressions found. The two new concerns (#1 and #2) are low-risk but worth noting — #2 in particular is a simple one-line fix (if (!signal?.aborted && currentAnimations.length > 0 && ...)).

[atomiks]

Codex Review (GPT-5.4)

This is a fresh correctness follow-up on a performance PR, updated with a full rerun of the requested perf slices on the latest pushed head 950329512 against upstream/master 3123f1e30. I still do not see any remaining blocking correctness issues; the current perf picture is mostly flat-to-favorable, with a small mixed result on the broader popup routes rather than a clean across-the-board win.

1. Remaining Bugs / Issues

No blocking or non-blocking correctness issues found in the latest pushed head.

2. Correctness Sweep

• Focus and interaction: the custom tabbable helper, FloatingFocusManager, FloatingPortal, Select, Menu, Popover, and detached-trigger paths now look internally consistent. The earlier MenuPopup initial-focus detour has been backed out, and the remaining focus behavior lines up with current master where it should.
• Render and lifecycle helpers: mergeProps, useRenderElement, and useAnimationsFinished now cover the previously risky cases directly. In particular, the first-getter mutation regression, rerender ref transitions, and the animation abort path all have matching code and tests again.
• State and selector changes: the createSelector split remains narrow, and the toast selector rewrite now preserves per-provider cache isolation via a WeakMap keyed per store instance.

3. Validation

• pnpm typescript
• pnpm test:jsdom tabbable FloatingFocusManager SelectPopup MenuRoot MenuCheckboxItem PopoverRoot mergeProps useToastManager useRenderElement useAnimationsFinished --no-watch
• pnpm test:chromium tabbable FloatingFocusManager SelectPopup MenuRoot MenuCheckboxItem PopoverRoot --no-watch
• pnpm test:jsdom SelectPopup MenuRoot MenuCheckboxItem Menubar PopoverRoot --no-watch
• pnpm test:chromium SelectPopup MenuRoot MenuCheckboxItem Menubar PopoverRoot --no-watch

For perf, I reran the latest matrix sequentially against exported production builds for the popup routes, alternating head / master one sample at a time to reduce order bias, then applied the same 1.5×IQR trimming rule after collection. I ran the non-popup Radio / Slider / Tabs sanity pass separately afterward with the same alternating-order rule so the route benchmarks and mount benchmarks were not competing for resources.

4. Refreshed Perf Sweep

Popup-route perf on exported production builds, 15 alternating samples per ref:

Route	PR head mean	PR head p99	master mean	master p99	Samples kept (head/master)	Readout
contained-triggers	117.44 ms	136.50 ms	116.47 ms	134.80 ms	15/15, 15/15	Essentially flat
detached-triggers	52.67 ms	58.70 ms	50.61 ms	57.30 ms	15/15, 15/15	Slightly slower on PR head
combobox-perf (auto index)	147.73 ms	153.80 ms	147.53 ms	156.00 ms	13/15, 15/15	Essentially flat
select-perf	165.47 ms	178.50 ms	164.34 ms	169.80 ms	15/15, 14/15	Essentially flat, slightly noisier tail on PR head

Non-popup sanity pass, using a lightweight alternating mount benchmark for 1000 items/instances per sample:

Benchmark	PR head mean	PR head p99	master mean	master p99	Samples kept (head/master)	Readout
radio x1000	536.30 ms	554.96 ms	533.00 ms	546.52 ms	14/15, 14/15	Essentially flat
slider x1000	639.93 ms	693.64 ms	645.42 ms	688.12 ms	15/15, 15/15	Slightly favorable to PR head
tabs x1000	97.86 ms	101.03 ms	98.38 ms	108.65 ms	15/15, 15/15	Slightly favorable to PR head

5. Earlier Supporting Microbenches

I did not rerun the internal tabbable() / getNextTabbable() / getPreviousTabbable() microbenches in this final pass, but the earlier supporting data from this review cycle was strongly favorable to the PR head there, and nothing in the latest branch state changed those helper hot paths materially.

Benchmark	Earlier PR-head mean	Earlier PR-head p99	Earlier master mean	Earlier master p99	Samples kept (head/master)	Readout
tabbable(container)	119.57 ms	123.05 ms	1490.20 ms	1513.09 ms	13/15, 15/15	Internal traversal was much faster on the PR head
getNextTabbable(reference)	99.45 ms	101.48 ms	363.81 ms	373.62 ms	15/15, 14/15	Internal traversal was much faster on the PR head
getPreviousTabbable(reference)	97.66 ms	99.85 ms	326.72 ms	339.08 ms	15/15, 15/15	Internal traversal was much faster on the PR head

6. Previous Review Issues Status

Previous issue	Status	Notes
mergeProps getter-first mutation	Fixed	The first getter result is owned again, with regression coverage.
useAnimationsFinished abort retry path	Fixed	The retry path now bails when already aborted.
Toast selector multi-provider isolation	Fixed	The local WeakMap cache preserves provider isolation and now has explicit coverage.
useRenderElement rerender coverage gap	Fixed	Rerender-driven coverage was added for enabled toggles and single-ref/array-ref transitions.
MenuPopup custom focus regression	Fixed	The initialFocus experiment and its stale branch-only test were both reverted.

Confidence

4/5. This pass covered the full branch again against the latest fetched base, refreshed the requested perf slices on the latest pushed head, and retained focused JSDOM/Chromium validation on the highest-risk interaction paths. Residual risk is still mostly in browser-specific focus edge cases around the custom tabbable helper, but the branch now has materially better regression coverage there than it did earlier in the review cycle.

[romgrk]

Inlining createSelectorMemoized feels wrong?

[atomiks]

I thought so too but it wins 1.5 KB for toast-only imports:

So this one change is worth about 1.4-1.5 KB gz, which is pretty meaningful for a single local refactor.

Why it saves that much:

putting createSelectorMemoized back in store.ts pulls the heavier reselect path back into the toast bundle the current code only needs a very small per-store WeakMap cache, so the bespoke version is much cheaper

[romgrk]

How common are toast-only imports? Is that a real use-case?

If it's a real use-case, this could instead be an opportunity to create a createMemoizedSelectorLite and put that code where it belongs (with the Store code, so if there is an implementation change, it's modified at the same time).

[atomiks]

I generally think it's good for each component to only have as much code as it really needs to. It makes treeshaking as effective as possible throughout the codebase. And yeah I think it's a real use case given you can use any number of Base UI components you want.

I'll look into the Lite idea

[atomiks]

I'll look into the Lite idea

I think this is a good follow-up idea but not worth the abstraction for this PR because only Toast uses it. It'd be valid to do so if we need it elsewhere later on.

[atomiks]

see that the store-based comps are popular enough to usually be used somewhere in an app anyway

Actually I was mistaken about the import graph here. It's only Toast (and Calendar later) that uses the reselect dep. So it pulls in 1.5 kB which increases the root bundle as well. It seems like reselect is needlessly heavy for our use cases.

With the "Lite" implementation:

Bundle	Gzip size
@base-ui/react	▼-3.3KB

Without

Bundle	Gzip size
@base-ui/react	▼-1.7KB

[flaviendelangle]

I think we will have more and more components that use the createSelectorMemoized.
So if we can make it lighter, that's awesome 👌

[atomiks]

Just experimented with this, and Codex wrote one faster than reselect in some cases but not in others (5 selectors, which Calendar has). So I guess because Calendar needs it, we'll have to leave it in the bundle.

[atomiks]

Bundle size decreases are still better than 1.7 KB if you don't use Toast (or later Calendar):

https://frontend-public.mui.com/size-comparison/mui/base-ui/diff?prNumber=4336&baseRef=master&baseCommit=9ebce1a886ce9eb69fc49e1b372cec66884b3ba9&headCommit=fd95a435f78269040ff859a19d9c77bd50f3867b

[romgrk]

It sounds like we can make wins on the reselect front, but it feels like those improvements could be left as a follow-up PR considering they require more examination & investigation.

[romgrk]

Feedback on the generated code: there are substantial issues with some of the changes here. Bundle-size is optimized at the cost of readability, performance and maintainability.

[atomiks]

@romgrk addressed the review comments

(Also with the useRenderElement changes I couldn't detect a difference in runtime perf running benchmarks locally with Codex. But I reverted them anyway as bundle size difference is negligible).