[Grid] Generify props with component property #16590
Conversation
fix TypeScript check error.
|
No bundle size changes comparing f3383e8...d7cc424 |
oliviertassinari
added
component: Grid
oliviertassinari
changed the title
|
@JipingWang Thanks for reporting the issue! |
| @@ -61,7 +61,7 @@ export default function SignInSide() { | |||
| <Grid container component="main" className={classes.root}> | |||
| <CssBaseline /> | |||
| <Grid item xs={false} sm={4} md={7} className={classes.image} /> | |||
| <Grid item xs={12} sm={8} md={5} component={() => <Paper square elevation={6} />}> | |||
| <Grid item xs={12} sm={8} md={5} component={Paper} elevation={6} square> | |||
There was a problem hiding this comment.
Just a random thought but would it maybe make more sense to push towards something like
function GridPaper() {
// but something specific here e.g. const className = useGrid(); or const css = useGrid()
const props = useGrid({ item, xs: 12 });
return <Paper {...props} />
}The current approach tends to grow quite nasty (member hocs?) with regards to what props belongs to what e.g. <Grid item elevation={12] component={Paper} /> (is elevation a Paper or Grid prop?).
IMO components should only ever accept strings for host components. There are a few exceptions like Links but I'd hope these will be replace with hooks in the future.
There was a problem hiding this comment.
Some thought:
- useGrid requires a hook ready CSS-in-JS solution.
- This example doesn't need to use the
componentprop. We could refactor it. (+1 dom node) - This example is lazy, it could create an intermediary component.
There was a problem hiding this comment.
Yeah I'm not really sure about the best API. IMO this <Render component={Something} /> pattern is just disguised hoc. I might've been a good idea for router links but I think it got abused1 beyond that.
1 At least now we might be able to provide a better API with hooks.
fix TypeScript check error.