[accordion][tabs] Make `value` a required prop

[michaldudak]

Made value in Accordion.Item, Tabs.Tab, and Tabs.Panel a mandatory prop. This helps to solve several issues with these components:

• closes #1880
• closes #1588
• closes #2096

[pkg-pr-new[bot]]

Open in StackBlitz

npm i https://pkg.pr.new/@base-ui-components/react@2124

commit: f91f634

[mui-bot]

Bundle size report

@base-ui-components/react parsed: ▼-523B^(-0.17%) gzip: ▼-149B^(-0.15%)

Show details for 39 more bundles

@base-ui-components/react/tabs parsed: ▼-521B^(-2.01%) gzip: ▼-153B^(-1.64%) @base-ui-components/react/accordion parsed: ▼-7B^(-0.03%) gzip: 0B^(0.00%) @base-ui-components/react/alert-dialog parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/avatar parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/checkbox parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/checkbox-group parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/collapsible parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/context-menu parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/dialog parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/direction-provider parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/field parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/fieldset parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/form parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/input parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/menu parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/menubar parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/merge-props parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/meter parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/navigation-menu parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/number-field parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/popover parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/preview-card parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/progress parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/radio parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/radio-group parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/scroll-area parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/select parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/separator parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/slider parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/switch parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/toast parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/toggle parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/toggle-group parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/toolbar parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/tooltip parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/unstable-no-ssr parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/unstable-use-media-query parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/use-render parsed: 0B^(0.00%) gzip: 0B^(0.00%) @base-ui-components/react/utils parsed: 0B^(0.00%) gzip: 0B^(0.00%)

Details of bundle changes

Generated by :no_entry_sign: dangerJS against f91f6340a4f0ce0e4a87c002a26ed46a2f9576fe

[netlify[bot]]

✅ Deploy Preview for base-ui ready!

Name	Link
🔨 Latest commit	f91f6340a4f0ce0e4a87c002a26ed46a2f9576fe
🔍 Latest deploy log	https://app.netlify.com/projects/base-ui/deploys/68556063d05a530008494aaa
😎 Deploy Preview	https://deploy-preview-2124--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.

[atomiks]

Would be good to add tests for each of those cases

[michaldudak]

Updated.

[atomiks]

NavigationMenu uses useId() inside of its Item component as a fallback when value is missing. I wonder if something similar could be used without requiring value

https://github.com/mui/base-ui/blob/8ba89eadb183833239310328ccf98af2953e17d5/packages/react/src/navigation-menu/item/NavigationMenuItem.tsx#L20-L21

[michaldudak]

useBaseUiId can be undefined before effects run (in React 17) and we need to have this value defined during SSR to get the tab panel content in the prerendered response. NavigationMenu doesn't need to be rendered on the server, so useBaseUiId is sufficient.

[atomiks]

useBaseUiId can be undefined before effects run (in React 17) and we need to have this value defined during SSR to get the tab panel content in the prerendered response.

So it's only a problem on React 17? I don't like that we need to change to a worse API to support a version of React that's fast becoming irrelevant; it's not future-looking. I'd rather change the type to be optional and just throw a runtime warning for React 17.

[mj12albert]

With useId is it possible to "fully" unmount the TabPanel when keepMounted={false}? https://github.com/mui/base-ui/issues/1872

[michaldudak]

It's not just that. The value must be possible to be figured out by developer so it can be used in defaultValue or value props. There's no way to do it with useId - the only sensible options are explicit value or index.

This approach is possible in NavigationMenu as defaultValue will often be null. In Tabs it's not common to see no tabs selected. Even in NavigationMenu you should provide explicit values if you want to control its state from the outside, right?

[atomiks]

It's not just that. The value must be possible to be figured out by developer so it can be used in defaultValue or value props. There's no way to do it with useId - the only sensible options are explicit value or index.

Yeah but imo it seems intuitive/expected that the value prop becomes required when you need to control it or select an initial tab. For Accordion this would also be less common than Tabs I imagine.

This approach is possible in NavigationMenu as defaultValue will often be null. In Tabs it's not common to see no tabs selected. Even in NavigationMenu you should provide explicit values if you want to control its state from the outside, right?

Yes, though it seems like with Tabs defaulting to the first tab is ok in many cases and you don't need to control it?

[ciampo]

I think that defaulting to the first (non disabled) tab being selected feels reasonable, if no value (or defaultValue) are passed.

[michaldudak]

Closed in #3373