[Tabs] Replace `cloneElement` with Context API to support custom and wrapped Tab components #46333

ZeeshanTamboli · 2025-06-13T07:13:51Z

Replace cloneElement with React's Context API without any breaking changes. This closes the following issues:

Tabs doesn't work with Tab as a custom component: Closes [Tabs] Tabs doesn't work with Tab as a custom component #27947
- Before: https://codesandbox.io/p/sandbox/material-demo-forked-gyjsdz
- After: https://codesandbox.io/p/devbox/material-demo-forked-mvlx5p
Remove cloneElement: Closes [material-ui][Tabs] Implement without cloneElement #28064
Conditional rendering of Tab not working: Closes [Tabs] Conditionally rendering of Tab not working properly #34740
- Before: https://codesandbox.io/p/sandbox/nervous-rgb-jtxlzf
- After: https://codesandbox.io/p/devbox/quiet-surf-3pzkyf
Using Divider causes console warnings: Closes [Divider][Tabs] Using divider with tab causes 4 console warnings in Chrome DevTools #38516
- Before: https://codesandbox.io/p/sandbox/ecstatic-fast-rhkzvn
- After: https://codesandbox.io/p/devbox/quizzical-cannon-khwt3z
Tooltip wrapping a Tab does not work: Closes [Tab] Regression for tooltip wrapping a tab #46265
- Before: https://codesandbox.io/p/devbox/material-demo-forked-hmv7s6
- After: https://codesandbox.io/p/devbox/material-demo-forked-cgt6qy

mui-bot · 2025-06-13T07:15:02Z

Netlify deploy preview

https://deploy-preview-46333--material-ui.netlify.app/

Bundle size report

Bundle	Parsed Size	Gzip Size
@mui/material	🔺+617B^(+0.12%)	🔺+297B^(+0.20%)
@mui/lab	🔺+169B^(+0.13%)	🔺+89B^(+0.22%)
@mui/system	0B^(0.00%)	0B^(0.00%)
@mui/utils	0B^(0.00%)	0B^(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against 6456107

sai6855 · 2025-06-17T06:00:10Z

packages/mui-material/src/Tab/Tab.js

+    if (!hasRegisteredRef.current) {
+      hasRegisteredRef.current = true;


function block inside useState hook runs only once through out component lifecycle (it doesn't run when component re-renders). So curious to understand the purpose of hasRegisteredRef, as it will always be false when this code runs

You're right that the initializer inside useState only runs once per mount. However, in React’s Strict Mode (development only), it's intentionally called twice to detect impure logic. Since registerTab mutates internal state, calling it twice would incorrectly register the tab multiple times, shifting tab indices and breaking the selection or indicator logic.

To avoid this, we guard it with hasRegisteredRef, ensuring registerTab runs only once — even in development. In production, the guard has no effect because the initializer runs only once as expected.

Ideally, the initializer should be pure (per React docs), but we intentionally break that rule here to support SSR — specifically, to precompute tab metadata so that the correct tab is marked selected on the first render (see test case). Without it, we get hydration mismatches..

I considered making registerTab idempotent, but that’s not feasible when we need to assign an implicit value based on the tab's render order. That requires incrementing a shared index counter (childIndexRef) — and we can’t require users to always provide explicit values to Tab without introducing a breaking change.

This approach strikes a balance: it ensures SSR correctness, avoids hydration issues, and works with wrapper components like <Tooltip><Tab /></Tooltip>, while remaining safe under React’s development behavior.

Open to suggestions if you think there's a cleaner way to achieve this.

If I understand it correctly this won't work well if you remove a tab dynamically (as there's no unregister function)

If I understand it correctly this won't work well if you remove a tab dynamically (as there's no unregister function)

It won't. But it isn't supported even in latest version.

This PR: https://stackblitz.com/edit/ry4fan5c-t3b4771r
Master: https://stackblitz.com/edit/ry4fan5c-oqugmytq

Using the register pattern without an unregister function sounds to me like it can introduce weird edge cases. Did you consider registering/unregistering on an effect instead?

Because we want to return the unregistering callback from the effect, so it's run on unmount.

But wouldn't the useEffect's setup function do nothing? It would simply return the output given as an input always when doing registerTab(finalValue).

This is not an option for us unless we want to wait for a new major.

Yes, not an option now.

We're already supporting implicit value, aren't we? With or without my suggestion.

Yes, we are already. Just wanted to point out some issues. I thought it would help us to understand.

But wouldn't the useEffect's setup function do nothing?

Only on the first run of the effect. Subsequent runs would unregister (cleanup) and register back. This is the same as having register/unregister in an effect, except that the very first time we already registered inside useState. I don't see an issue with it if register is idempotent 🤔

I tried doing this in 7221ea8 but the tests fail. Any idea why? However, it works locally on browser.

Which test failed? What was the message?

Tests pass now. The tests run with Strict Mode, which helped me catch the issue that I wasn’t decrementing the child index on tab unregistration.

DiegoAndai

Thanks for working on this @ZeeshanTamboli, it looks to me like a clear improvement on the implementation.

I added some initial questions.

When we release this, we should do it under a minor version just to be cautious about changes that might be perceived as breaking.

Did you add test for all the issues this would close?

packages/mui-material/src/Tabs/Tabs.js

packages/mui-material/src/Tab/Tab.js

packages/mui-material/src/Tabs/TabsContext.ts

ZeeshanTamboli · 2025-06-18T14:55:33Z

@ZeeshanTamboli I think this change (225cafd) needs to be reverted.

In previous implementation, when onChange is added to both Tabs and Tab, onChange of Tabs always overrides onChange of Tab, check here for implementation =>

material-ui/packages/mui-material/src/Tabs/Tabs.js

Line 809 in 4ec315e

onChange,

In this version, onChange of Tabs doesn't override onChange of Tab thus resulting in broken behaviour of Tabs when both Tabs and Tab has onChange handler.

Check this example built on top of this PR: https://stackblitz.com/edit/ry4fan5c?file=src%2FDemo.tsx, try to change tabs it doesn't work.

I'm aware this is extremely niche use case, not sure if it's really a valid use case, but just wanted to bring it your attention

👍 Reverted. I think there is no breaking change then cc @DiegoAndai

DiegoAndai · 2025-06-18T19:33:45Z

@ZeeshanTamboli I agree with @sai6855 that onChange behavior shouldn't change: when onChange is added to both Tabs and Tab, onChange of Tabs overrides the one on Tab. We should keep that, but we shouldn't remove the prop from Tab. Does this make sense?

ZeeshanTamboli · 2025-06-19T07:21:04Z

@ZeeshanTamboli I agree with @sai6855 that onChange behavior shouldn't change: when onChange is added to both Tabs and Tab, onChange of Tabs overrides the one on Tab. We should keep that, but we shouldn't remove the prop from Tab. Does this make sense?

@DiegoAndai I think we can safely remove the onChange prop from Tab.

In both the current version (master) and this PR:

✅ If onChange is provided only to <Tabs>, it’s called.
❌ If onChange is provided only to <Tab>, it’s not called. The undefined onChange from Tabs takes priority. And also <Tab> cannot be used as a standalone component without <Tabs>.
✅ If provided to both, the one from <Tabs> takes priority.

You can see this behavior is consistent in both versions:

Master: https://stackblitz.com/edit/ry4fan5c-3goxgph6
This PR: https://stackblitz.com/edit/ry4fan5c-r5tnuyfm

packages/mui-material/src/Tabs/Tabs.js

ZeeshanTamboli · 2025-06-19T08:42:14Z

@ZeeshanTamboli I think we should add additional tests for:

[Tabs] Tabs doesn't work with Tab as a custom component #27947

[Tabs] Conditionally rendering of Tab not working properly #34740

[Divider][Tabs] Using divider with tab causes 4 console warnings in Chrome DevTools #38516

(The one we already have covers #46265)

@DiegoAndai Addressed in f47c38e

sai6855

LGTM

michaldudak · 2025-06-20T08:05:11Z

packages/mui-material/src/Tabs/Tabs.js

@@ -375,7 +376,7 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) {
    scrollbarWidth: 0,
  });

-  const valueToIndex = React.useRef(new Map()).current;
+  const valueToIndex = useLazyRef(() => new Map()).current;


() => new Map() can be extracted to a function in the module scope so a new one isn't instantiated every time this code runs.

Done in acc42c2

siriwatknp

👏 Great job, let's refine the Tabs context fields and add docs.

siriwatknp · 2025-06-26T02:39:26Z

packages/mui-material/src/Tabs/TabsContext.ts

+  selectionFollowsFocus?: TabsProps['selectionFollowsFocus'];
+  onChange?: TabsProps['onChange'];
+  textColor?: TabsProps['textColor'];
+  tabsValue?: TabsProps['value'];


Suggested change

tabsValue?: TabsProps['value'];

value?: TabsProps['value'];

I think it's better to use value here, it's a standard name with onChange. The TabsContext will need to be exposed to public.

siriwatknp · 2025-06-26T02:43:06Z

packages/mui-material/src/Tabs/TabsContext.ts

+  registerTab?: (tabValue: TabProps['value']) => {
+    finalValue: TabProps['value'];
+    assignedIndex: number;
+  };


Suggested change

registerTab?: (tabValue: TabProps['value']) => {

finalValue: TabProps['value'];

assignedIndex: number;

};

privateRegisterTab?: (tabValue: TabProps['value']) => {

finalValue: TabProps['value'];

assignedIndex: number;

};

I am not sure about this, ignore this suggestion if you think user can build a custom Tab with this API.

siriwatknp

I think we need to be clear on https://mui.com/material-ui/react-tabs/#experimental-api.

In my opinion, we should communicate that user can move away from the lab to this new context API.

We should think about this a bit more. I am struggle on what's the structure should be, ideally I want:

<Tabs>
  <TabList>
    <Tab>
    …
  </TabList>
  <TabPanel>…</TabPanel>
  <TabPanel>…</TabPanel>
  <TabPanel>…</TabPanel>
</Tabs>

but not sure if this is possible given the existing Tabs we have.

Probably the best option is to use new prop to change the layout:

<Tabs
  panels={
    <div>
      <TabPanel>…</TabPanel>
      <TabPanel>…</TabPanel>
      <TabPanel>…</TabPanel>
    </div>
  }
>
  <Tab>…</Tab>
</Tabs>

@ZeeshanTamboli here is my proposal ZeeshanTamboli#3

…thub.com/ZeeshanTamboli/material-ui into tabs-remove-cloneElement-fix-issue-46265

ZeeshanTamboli · 2025-06-26T13:34:33Z

We should think about this a bit more. I am struggle on what's the structure should be, ideally I want:
<Tabs>
  <TabList>
    <Tab>
    …
  </TabList>
  <TabPanel>…</TabPanel>
  <TabPanel>…</TabPanel>
  <TabPanel>…</TabPanel>
</Tabs>

@siriwatknp I think we can do this. We can use the context from Tabs by setting idPrefix in the Tabs context. Then, we can apply aria-controls and id on each Tab, and aria-labelledby and id on eachTabPanels.

Then we can remove the need of the TabContext component from Lab altogether and move TabList and TabPanel components to Material UI.

I think we can even remove TabList. Just have Tabs, Tab and TabPanel with full accessibility support.

siriwatknp · 2025-06-26T15:06:32Z

@siriwatknp I think we can do this. We can use the context from Tabs by setting idPrefix in the Tabs context. Then, we can apply aria-controls and id on each Tab, and aria-labelledby and id on eachTabPanels.

Then we can remove the need of the TabContext component from Lab altogether and move TabList and TabPanel components to Material UI.

I think we can even remove TabList. Just have Tabs, Tab and TabPanel with full accessibility support.

In terms of logic I agree, but for the layout, what's your plan to make it work without a breaking change?

The existing Tabs passes the children inside the scroller, so this would not work:

<Tabs>
  <Tab></Tab>
  <Tab></Tab>
  <Tab></Tab>
  <TabPanel></TabPanel>
  <TabPanel></TabPanel>
  <TabPanel></TabPanel>
</Tabs>

ZeeshanTamboli · 2025-06-28T09:29:57Z

I think we need to be clear on https://mui.com/material-ui/react-tabs/#experimental-api.

In my opinion, we should communicate that user can move away from the lab to this new context API.

We should think about this a bit more. I am struggle on what's the structure should be, ideally I want:
<Tabs>
  <TabList>
    <Tab>
    …
  </TabList>
  <TabPanel>…</TabPanel>
  <TabPanel>…</TabPanel>
  <TabPanel>…</TabPanel>
</Tabs>
but not sure if this is possible given the existing Tabs we have.

Probably the best option is to use new prop to change the layout:
<Tabs
  panels={
    <div>
      <TabPanel>…</TabPanel>
      <TabPanel>…</TabPanel>
      <TabPanel>…</TabPanel>
    </div>
  }
>
  <Tab>…</Tab>
</Tabs>
@ZeeshanTamboli here is my proposal ZeeshanTamboli#3

@siriwatknp Thanks for sharing the proposal. There are a lot of file changes so I didn't see it thoroughly.

That said, I think this PR should focus only on fixing the issues with Tabs and Tab, as mentioned in the description.

Supporting TabPanel directly inside Tabs (because now it has the Context) would require changing the API (like your suggested layout), which makes sense but might be better handled in a separate PR — or ideally in a future major version where we can fully and freely refactor the Tabs structure. For now, the Lab components still work well for TabPanel.

This PR is just about improving the core behavior when using Tabs and Tab alone.

ZeeshanTamboli added 2 commits June 12, 2025 22:21

initial implementation

0222cf6

support implicit child position index selection

5dc33fa

ZeeshanTamboli added component: tabs This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature labels Jun 13, 2025

ZeeshanTamboli changed the title ~~[WIP] Tabs remove clone element fix issue 46265~~ [WIP] Tabs remove clone element Jun 13, 2025

ZeeshanTamboli added 17 commits June 13, 2025 12:52

update proptypes

386da19

fix lint

3d973d6

Merge branch 'master' into tabs-remove-cloneElement-fix-issue-46265

6ddd67c

fix lint

e3ec564

make some tests pass

26e871e

register tab on initial render

e356f19

focus on first tab if tabs value is false

afa4db4

pnpm proptypes

2129348

Merge branch 'master' into tabs-remove-cloneElement-fix-issue-46265

4ea85d7

fix tab tests

b5b64cd

fix lint

7614a26

remove async

ec7e32d

revert adding cross-env

fb1ef58

Merge branch 'master' into tabs-remove-cloneElement-fix-issue-46265

49f17fd

Add TabsContextType and useTabsContext hook

973ab5f

Added test

1457d4b

add test to assert if tab is rendered without Tabs

6e73e8d

ZeeshanTamboli changed the title ~~[WIP] Tabs remove clone element~~ [Tabs] Remove clone element Jun 16, 2025

ZeeshanTamboli changed the title ~~[Tabs] Remove clone element~~ [Tabs] Replace cloneElement with Context API to support custom and wrapped Tab components Jun 16, 2025

ZeeshanTamboli marked this pull request as ready for review June 16, 2025 16:51

ZeeshanTamboli requested review from DiegoAndai and sai6855 June 16, 2025 16:53

sai6855 reviewed Jun 17, 2025

View reviewed changes

DiegoAndai reviewed Jun 17, 2025

View reviewed changes

packages/mui-material/src/Tabs/Tabs.js Outdated Show resolved Hide resolved

packages/mui-material/src/Tab/Tab.js Show resolved Hide resolved

packages/mui-material/src/Tabs/TabsContext.ts Outdated Show resolved Hide resolved

michaldudak reviewed Jun 19, 2025

View reviewed changes

packages/mui-material/src/Tabs/Tabs.js Outdated Show resolved Hide resolved

ZeeshanTamboli added 3 commits June 19, 2025 12:59

Merge branch 'master' into tabs-remove-cloneElement-fix-issue-46265

f7e96da

Add tests for each issue

f47c38e

use React.Fragment

f2f6739

ZeeshanTamboli added 2 commits June 19, 2025 18:11

use useLazyRef for better performance

0ba679d

Merge branch 'master' into tabs-remove-cloneElement-fix-issue-46265

526079f

ZeeshanTamboli requested review from sai6855 and michaldudak June 19, 2025 12:56

sai6855 approved these changes Jun 19, 2025

View reviewed changes

michaldudak reviewed Jun 20, 2025

View reviewed changes

ZeeshanTamboli added 2 commits June 20, 2025 18:05

extract new Map into a function

acc42c2

Merge branch 'master' into tabs-remove-cloneElement-fix-issue-46265

31ebdea

ZeeshanTamboli requested a review from michaldudak June 20, 2025 12:49

DiegoAndai mentioned this pull request Jun 23, 2025

[material-ui][ButtonGroup] Renders incorrectly when rendering conditionally from custom component. #39488

Open

2 tasks

siriwatknp reviewed Jun 26, 2025

View reviewed changes

siriwatknp requested changes Jun 26, 2025

View reviewed changes

siriwatknp and others added 3 commits June 26, 2025 11:04

Merge branch 'master' into tabs-remove-cloneElement-fix-issue-46265

651fdd2

register/unregister in effect

7221ea8

Merge branch 'tabs-remove-cloneElement-fix-issue-46265' of https://gi…

d15b8a2

…thub.com/ZeeshanTamboli/material-ui into tabs-remove-cloneElement-fix-issue-46265

ZeeshanTamboli added 2 commits June 27, 2025 20:42

decrease child index on unregister

98cb569

Merge branch 'master' into tabs-remove-cloneElement-fix-issue-46265

6456107

		if (!hasRegisteredRef.current) {
		hasRegisteredRef.current = true;

Uh oh!

[Tabs] Replace cloneElement with Context API to support custom and wrapped Tab components #46333

Are you sure you want to change the base?

[Tabs] Replace cloneElement with Context API to support custom and wrapped Tab components #46333

Conversation

ZeeshanTamboli commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mui-bot commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Netlify deploy preview

Bundle size report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZeeshanTamboli Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZeeshanTamboli Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DiegoAndai Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZeeshanTamboli Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DiegoAndai left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZeeshanTamboli commented Jun 18, 2025

Uh oh!

DiegoAndai commented Jun 18, 2025

Uh oh!

ZeeshanTamboli commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ZeeshanTamboli commented Jun 19, 2025

Uh oh!

sai6855 left a comment

Choose a reason for hiding this comment

Uh oh!

michaldudak Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

siriwatknp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

siriwatknp left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZeeshanTamboli commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

[Tabs] Replace `cloneElement` with Context API to support custom and wrapped Tab components #46333

[Tabs] Replace `cloneElement` with Context API to support custom and wrapped Tab components #46333

ZeeshanTamboli commented Jun 13, 2025 •

edited

Loading

mui-bot commented Jun 13, 2025 •

edited

Loading

ZeeshanTamboli Jun 18, 2025 •

edited

Loading

ZeeshanTamboli Jun 19, 2025 •

edited

Loading

DiegoAndai Jun 20, 2025 •

edited

Loading

ZeeshanTamboli Jun 25, 2025 •

edited

Loading

DiegoAndai left a comment •

edited

Loading

ZeeshanTamboli commented Jun 19, 2025 •

edited

Loading

michaldudak Jun 20, 2025 •

edited

Loading

siriwatknp left a comment •

edited

Loading

ZeeshanTamboli commented Jun 26, 2025 •

edited

Loading

siriwatknp commented Jun 26, 2025 •

edited

Loading

ZeeshanTamboli commented Jun 28, 2025 •

edited

Loading