[utils] Support RSC in isMuiElement util
#38129
Conversation
Netlify deploy previewhttps://deploy-preview-38129--material-ui.netlify.app/ Bundle size reportDetails of bundle changes (Toolpad) |
sai6855
changed the title
sai6855
changed the title
sai6855
changed the title
sai6855
changed the title
| React.isValidElement(child) && | ||
| (isMuiElement(child, ['Grid']) || | ||
| // eslint-disable-next-line no-underscore-dangle | ||
| (child.type as any)?._payload?.value?.muiName === 'Grid') |
There was a problem hiding this comment.
The fix works for me in a sandbox: https://codesandbox.io/p/sandbox/https-github-com-mui-material-ui-issues-37932-forked-qtf5j2?file=%2Fapp%2Fclient%2Fpage.tsx%3A9%2C1 (I gave the CI a kick)
Maybe we could add this additional check directly inside isMuiElement? It does increase the scope of the PR but so far only 8 Joy UI components (in addition to Grid) uses this
Without it other issues occur in a Next.js server component page when it's used, e.g. this usage of Joy UI Typography + Skeleton – In the /server page, the Skeleton doesn't show without manually adding the variant prop: https://codesandbox.io/p/sandbox/https-github-com-mui-material-ui-pull-38129-forked-93y266?file=%2Fapp%2Fserver%2Fpage.tsx%3A15%2C1 CC @siriwatknp & @brijeshb42
There was a problem hiding this comment.
This issue could be automatically fixed if logic is moved to isMuiElement function. #38791
There was a problem hiding this comment.
Where does the _payload object come from? Is it related to generic server rendered component or is it specific to nextjs?
There was a problem hiding this comment.
I'm not entirely sure from where _payload is coming. searched both nextjs and react docs neither of them have any info on this.
The most relavent info i could find in nextjs docs is this : https://nextjs.org/docs/app/building-your-application/rendering/server-components#how-are-server-components-rendered
There was a problem hiding this comment.
Probably related to this.
There was a problem hiding this comment.
ah yes, so it's react related
There was a problem hiding this comment.
@sai6855 I think it's better to fix the isMuiElement util in this case. As a note, the muiName should be replaced with CSS (likely :has selector) but the browser support is not wide enough.
There was a problem hiding this comment.
Also, please add a comment about child.type as any)?._payload?.value?.muiName === 'Grid' with the link @brijeshb42 pointed to.
There was a problem hiding this comment.
@siriwatknp made suggested changes
sai6855
requested review from
siriwatknp
and removed request for
siriwatknp
sai6855
force-pushed
the
placement-demo
branch
2 times, most recently
from
8802d98
to
6ff8b11
Compare
sai6855
force-pushed
the
placement-demo
branch
from
6ff8b11
to
738b27e
Compare
|
The changes looks good, however, please rename the title to match more specifically the changes done. |
sai6855
changed the title
sai6855
changed the title
isMuiElement util
mnajdova
changed the title
isMuiElement utilisMuiElement util
closes: #37932
before: https://codesandbox.io/p/sandbox/https-github-com-mui-material-ui-issues-37932-forked-r2dm4w?file=%2Fpackage.json%3A16%2C30
after: https://codesandbox.io/p/sandbox/https-github-com-mui-material-ui-issues-37932-forked-9pm6nn
reason for issue: #37932 (comment)