-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[utils] skip deep clone React element #44400
Conversation
Netlify deploy previewhttps://deploy-preview-44400--material-ui.netlify.app/ @material-ui/core: parsed: -0.10% 😍, gzip: -0.05% 😍 Bundle size reportDetails of bundle changes (Toolpad) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skipping React elements in deepClone
and deepMerge
makes sense to me 👍🏼
Because this is a shared util, let's wait to see if @mui/x or @mui/toolpad have any objections. If there are none in a couple of days, we can go ahead and merge this.
Does that sound right?
We are not using either of those (unless they are used under the hood in some API we use) so for me it's good |
Thanks for the info @flaviendelangle @bharatkashyap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siriwatknp looks like we're good to go, let's merge 😊
@siriwatknp can you merge this fix into mui v5? There's the same issue. |
closes #44278
Root cause
Next.js 15 with Turbopack has a different signature for React element which cause the
@mui/utils/deepmerge
to recursively deepclone React element. More details on #44278Fix
I think it does not make sense anyway for the deepmerge function to deepClone React elements.
So I added a check to deepmerge directly to avoid deepClone React elements.
Tested with https://github.com/nphmuller/mui-next-15_0_2_issue, this PR fixes the issue.