[DataGrid] Rendered fewer hooks than expected

[oliviertassinari]

• [x] The issue is present in the latest release.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Rendering a React component as part of the renderCell: API creates issues. This is a render function

Expected Behavior 🤔

Developers should get a short feedback loop that their usage of the grid API is wrong.

Steps to Reproduce 🕹

Steps:

1. https://codesandbox.io/s/data-grid-popover-bug-1kcff?fontsize=14&hidenavigation=1&theme=dark

Context 🔦

This is a follow-up on #1376 where we didn't leverage the feedback as much as we could.

Your Environment 🌎

v4.0.0-alpha.24

[oliviertassinari]

Regarding the solution, we can't introduce a new warning as React doesn't provide any primitives to know (I'm aware off) if a function is a component or a render function. Instead, I propose we update the documentation:

diff --git a/docs/src/pages/components/data-grid/rendering/rendering.md b/docs/src/pages/components/data-grid/rendering/rendering.md
index 0ebaf4ef..629990ca 100644
--- a/docs/src/pages/components/data-grid/rendering/rendering.md
+++ b/docs/src/pages/components/data-grid/rendering/rendering.md
@@ -101,6 +101,12 @@ const columns: GridColDef[] = [

 {{"demo": "pages/components/data-grid/rendering/RenderCellGrid.js", "defaultCodeOpen": false, "bg": "inline"}}

+> ⚠️ The `renderCell` property expects a render function.
+> You can't provide a component directly.
+> If you do, you can expect errors like this `Rendered fewer hooks than expected` to throw.
+>
+> To use a component, do: `renderCell: () => <MyComponent />`.
+
 #### Expand cell renderer

 By default, the grid cuts the content of a cell and renders an ellipsis if the content of the cell does not fit in the ce

[angelsvirkov]

@oliviertassinari, I have stumbled upon this issue, as well, and I was trying to find a way to solve it internally without the necessity for a warning or and extra care taken by users.

Here is how react-table is trying to figure out if it is dealing with a render function or a component. I am not sure if it can help you as I am not familiar with your internals but I think it is worth taking a look here:

• https://github.com/tannerlinsley/react-table/blob/8ec25cabccf915991ec00cf67bfb26a83aa0c0b2/src/hooks/useTable.js#L338
• https://github.com/tannerlinsley/react-table/blob/1fb9d553f5c7e921a52298b0590082a9f708301d/src/publicUtils.js#L216

Update: I found your comment and checked the demos that are failing with RenderExpandCellGrid. After I tried it out, it looks like the isReactComponent function from react-table is covering these cases in the demo, as well. I am referring to the custom components using React.memo. Here is a codesandbox example

[oliviertassinari]

@angelsvirkov The link to react-table is interesting. They are violating the notion of a render function by turning it into a React component, just in case. It's pragmatic. We could consider it as an alternative solution. It will cost a bit of performance for those that don't need a component but nothing I expect significantly.

I would personally propose:

1. We update the docs https://github.com/mui-org/material-ui-x/issues/1402#issuecomment-818317805
2. We keep this issue open, waiting for upvotes
3. If developers keep complaining, we leverage react-table's tradeoff.

[saskenuba]

Hello! Turning it into a react component would solve my current use-case. I am writing clojurescript with the reagent framework, and I can't find a way to make it return a ReactElement. Since you've said that the performance impact is not significant, perhaps just go the most pragmatic way? Thanks

[oliviertassinari]

perhaps just go the most pragmatic way?

@saskenuba Not so sure that it's pragmatic. We have been teaching developers that renderX means it's not a component. So far, it doesn't seem to impact too many developers. If we change this behavior, we would create surprises.

A new thought comes to mind. The main value of a render function is to make it easier for developers to create a new function at each render, accessing the outer JS scope, without having React unmount/mount the component because the reference changed. But in our case, we force developers to memorize the columns in the first place, so this renderCell API doesn't seem to make a lot of sense. A Cell component API could work better.

[himself65]

I wrap the component and it fixes

before:

    renderCell: Component,

after:

    renderCell: (props) => <Component {...props} />,

[ianschmitz]

I was able to reproduce this issue by resizing the table to a very small width - close to 0px but doesn't have to be exactly 0.

[magnusros]

~I am using pattern: renderCell: (props) => <Component {...props}/> , but still get this error when adding too many columns. When running on mobile phone browser it crashes with even fewer columns. Using x-data-grid-premium. Is there any limitation on number of columns?~

Update: Reason for the crash was that I used React.useCallback for caching.

[oliviertassinari]

@magnusros Do you have a reproduction?