Skip to content

Conversation

@mj12albert
Copy link
Member

@mj12albert mj12albert commented Nov 8, 2023

Closes #39474
Also see #39833 as related (for our demos in the docs that have a renderOption)

Demo: https://codesandbox.io/p/sandbox/https-gitproxy.zycloud.tk-mui-material-ui-pull-39795-fwdm7f?file=%2Fsrc%2Fapp%2Fpage.tsx%3A1%2C1

There should no console errors when opening the listbox with grouped options

Next.js doesn't like it when a key prop is part of an object being spread, here's the related issue in their repo: vercel/next.js#55642

BTW I'm not sure why this doesn't repro when the options are ungrouped, as in both cases they both use defaultRenderOption

@mj12albert mj12albert added typescript package: material-ui scope: autocomplete Changes related to the autocomplete. This includes ComboBox. nextjs labels Nov 8, 2023
@mui-bot
Copy link

mui-bot commented Nov 8, 2023

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 4cec2fc

@mj12albert mj12albert marked this pull request as ready for review November 8, 2023 07:57
@mj12albert mj12albert requested review from DiegoAndai, michaldudak and mnajdova and removed request for DiegoAndai November 8, 2023 07:58
@mnajdova mnajdova merged commit 13fec2d into mui:master Nov 8, 2023
@mj12albert mj12albert deleted the material/autocomplete-grouped branch November 8, 2023 10:09
Comment on lines +555 to +562
const defaultRenderOption = (props2, option) => {
const { key, ...otherProps } = props2;
return (
<li key={key} {...otherProps}>
{getOptionLabel(option)}
</li>
);
};
Copy link
Member

@oliviertassinari oliviertassinari Nov 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spread is not a very fast operator (speaking under @romgrk control, who I think did changes like this in the MUI X codebase to improve performance)

Would this be better?

Suggested change
const defaultRenderOption = (props2, option) => {
const { key, ...otherProps } = props2;
return (
<li key={key} {...otherProps}>
{getOptionLabel(option)}
</li>
);
};
const defaultRenderOption = (props2, option) => {
// Need to clearly apply key because of https://github.com/vercel/next.js/issues/55642
return (
<li key={props2.key} {...props2}>
{getOptionLabel(option)}
</li>
);
};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled in #40968

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nextjs scope: autocomplete Changes related to the autocomplete. This includes ComboBox. typescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[material-ui][Autocomplete] Grouped options demo has console errors in Next.js

5 participants