-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
[Grid] ignore RSC error #233
Conversation
Thanks for taking this @siriwatknp. The solution doesn't work for this case: https://mui.com/material-ui/react-grid2/#inheriting-columns. For that use case to work, a grid item must forward its parent column and spacing values, which won't work if we check for the container prop. What do you think about something like this?: function isGridComponent(element) {
try {
// For server components `muiName` is avaialble in element.type._payload.value.muiName
// relevant info - https://github.com/facebook/react/blob/2807d781a08db8e9873687fccc25c0f12b4fb3d4/packages/react/src/ReactLazy.js#L45
// eslint-disable-next-line no-underscore-dangle
return element.type.muiName === 'Grid' || element.type?._payload?.value?.muiName === 'Grid';
} catch {
// Covers for the case in which the parent is a server component and the child is a client component
// https://github.com/mui/material-ui/issues/43635
return false
};
} Would this work? It would fail silently, but maybe we can add a callout on the docs about this case. We should also catch only this specific error and not others. |
This comment was marked as outdated.
This comment was marked as outdated.
You proposal works! I will go with your proposal. |
9b4684a
to
ca31fb9
Compare
// relevant info - https://github.com/facebook/react/blob/2807d781a08db8e9873687fccc25c0f12b4fb3d4/packages/react/src/ReactLazy.js#L45 | ||
// eslint-disable-next-line no-underscore-dangle | ||
return element.type.muiName === 'Grid' || element.type?._payload?.value?.muiName === 'Grid'; | ||
} catch (error) { |
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.
Can we check for the specific error so we don't catch unintended ones?
if (error.message.startsWith("Cannot access default.muiName on the server.") {
// Covers for the case in which the Grid is a server component and the child is a client component
// https://github.com/mui/material-ui/issues/43635
return false;
} else {
throw error
}
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.
Are you sure, the message check is quite fragile. Given the size of the function, eventhough it catches, I think it's still better than checking message.
cc @brijeshb42
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.
Just speculating. Is there some way we can also give this control to the user so they can customize this if the error criteria changes in the future?
container
closes mui/material-ui#43635
This is a temporary fix, which might not cover an edge case if the consumer use client component as a child of Grid container.
However, this fix should work for normal use cases because a Grid container should always have Grid item.