base-ui icon indicating copy to clipboard operation
base-ui copied to clipboard

[Select] SSR empty render

Open oliviertassinari opened this issue 1 year ago • 5 comments

Duplicates

  • [X] I have searched the existing issues

Latest version

  • [X] I have tested the latest version

Steps to reproduce 🕹

Link to live example:

Steps:

  1. Open https://mui.com/base-ui/react-select/#introduction
  2. Disable JavaScript to see the initial html render before hydration

Current behavior 😯

Screenshot 2023-07-23 at 18 27 54

Expected behavior 🤔

Screenshot 2023-07-23 at 18 25 12

Context 🔦

This is a regression in behavior from Material UI, e.g. https://mui.com/material-ui/react-select/#basic-select, which we would need to fix for Material UI v6 to depend on Base UI.

I noticed this on the https://github.com/mui/material-ui/tree/master/examples/base-next-app-router-tailwind-ts which feels broken:

https://github.com/mui/material-ui/assets/3165635/b59ac464-6dc0-4b75-bb86-bd63696738cb

Joy UI is broken too.

cc @mj12albert as owning the select component: https://www.notion.so/mui-org/Components-Inputs-ecd48c9afe354bda8ecda3e96abc8c3c

Your environment 🌎

@mui/base v5.0.0-beta.8

oliviertassinari avatar Jul 23 '23 16:07 oliviertassinari

It's because useSelect uses the internal useCompound hook which relies on useEffect so it won't work with SSR: https://github.com/mui/material-ui/blob/master/packages/mui-base/src/utils/useCompound.ts#L96

mj12albert avatar Jul 31 '23 09:07 mj12albert

@mj12albert, that's correct - options are not known on the server. ~~It may be possible to work around this by allowing the Select to render its selected value even if it doesn't exist among the options.~~

EDIT: Actually, no, as the Select needs to know its Options to get a label associated with the selected value. So rendering a value without having the options collection can't happen. There is a possible workaround - instead of passing options as JSX elements, Select could also accept the options prop. This way, all the options are known during the first render.

I agree that not having the value rendered on the server is a downside (and a regression compared to Material UI), but the change that caused it enabled other requested features (such as being able to wrap options in custom components). If styled properly (ensuring that an empty select has the same dimensions as a filled one), this shouldn't be a major issue.

michaldudak avatar Jul 31 '23 09:07 michaldudak

Select could also accept the options prop. This way, all the options are known during the first render.

Would it be possible to achieve all the features + SSR if Select accepts an options prop and provides a render prop?

<Select options={[{ value: 1, label: 'One' }, { value: 2, label: 'Two' }]}>
  {(renderProps) => {
    return renderProps.options.map(option => (
      <Option key={option.value} value={option.value}>
        {option.label}
      </Option>
    ))
  }}
</Select>

React Spectrum has a similar API: https://react-spectrum.adobe.com/react-spectrum/Picker.html#content

mj12albert avatar Aug 03 '23 08:08 mj12albert

For how to solve this, as far as I understand it, we can approach it like in the Autocomplete. We can introduce a https://mui.com/material-ui/api/autocomplete/#Autocomplete-prop-getOptionLabel like prop. Well, it depend if value and option are the same type in Base UI Select.

Material UI Select has something related but different, renderValue: https://mui.com/material-ui/react-select/#chip.

Select could also accept the options prop

I have seen a number of issues in Material UI where developers render the Select value before fetching the option, where they are actually of different types. The options prop feels like we would ask more than we need.


Aside of this, there is also a bug with the demo, the component shouldn't collapse when the selected value label is "". In Material UI, I think we solve this with a no width space character.

oliviertassinari avatar Aug 03 '23 19:08 oliviertassinari

The options prop feels like we would ask more than we need

But then, strange, React Aria also does have this https://react-spectrum.adobe.com/react-aria/Select.html#dynamic-collections, this hides something, at least, I don't understand why they need it.


In Radix UI, it seems that you can solve this by rendering the value directly: https://www.radix-ui.com/docs/primitives/components/select#controlling-the-value-displayed-in-the-trigger when controlled. But when uncontrolled, this is broken too https://codesandbox.io/p/sandbox/busy-mccarthy-n2rq6h?file=%2Fapp%2Fpage.tsx%3A16%2C1.


Overall, I feel that we might want to prioritize fixes in this order:

  1. Fix the vertical layout shift in https://mui.com/base-ui/react-select/#introduction. For example,

https://github.com/mui/material-ui/blob/cddfa953eea57a6209cac19e321fcdb32ba908f0/packages/mui-material/src/Select/SelectInput.js#L507-L510

could be a clean solution. This is a problem during hydration but also when the value is empty. Proof:

Screenshot 2023-08-04 at 16 03 09
  1. Fix the horizontal layout shift in Base UI's example template.
  2. Allow developers to render a value even during hydration. This I think would be were we fix the Material UI integration.

We could solve 2. and 3. at the same time by rendering the value.

oliviertassinari avatar Aug 04 '23 13:08 oliviertassinari