[data grid] Re-mounting DataGrid with apiRef causes error (apiRef.current is null)

[philer-jambit]

Duplicates

• [X] I have searched the existing issues

Latest version

• [X] I have tested the latest version

Steps to reproduce 🕹

CodeSandbox: https://codesandbox.io/s/amazing-panini-4pyuru?file=/demo.tsx

I'm trying to use useGridApiRef with a <DataGridPro> that gets mounted conditionally. Here's the simplest example I could come up with:

const GridExample = () => {
  const apiRef = useGridApiRef()
  const [show, setShow] = useState(true)
  return (
    <div>
      <Button onClick={() => setShow(show => !show)}>toggle</Button>
      {show && (
        <DataGridPro
          apiRef={apiRef}
          autoHeight
          rows={[{ id: 1 }, { id: 2 }, { id: 3 }]}
          columns={[{ field: "id" }]}
        />
      )}
    </div>
  )
}

Current behavior 😯

After mounting, un-mounting and re-mounting the component, the following error is raised:

Warning: Failed prop type: The prop `apiRef.current` is marked as required in `ForwardRef(DataGridPro)`, but its value is `null`.

The fact that apiRef.current can be null also means that additional checks are necessary when accessing it, e.g. in an effect hook. This is not reflected by the corresponding type React.MutableRefObject<GridApiPro>.

Adding the following workaround solves the runtime issues:

  // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
  if (apiRef.current === null) {
    // @ts-expect-error {} is the initial value set by useGridApiRef
    apiRef.current = {}
  }

Expected behavior 🤔

Don't throw an error for the above code – it doesn't do anything crazy. To solve this, I assume one would have to

either:

Don't set apiRef.current to null – Note that setting an empty object, as the useGridApiRef hook does on init, might also cause issues as it does not match the DataGridPro type.

or:

Make null a legal value for apiRef.current and also reflect that in the typing of useGridApiRef.

Context 🔦

We retrieve data from our backend to render it in a table. Said request may fail, in which case we show an error message in place of the table. The apiRef is used to track density changes so we can persist them in localStorage.

Here's a rough sketch of what our code does:

const { data, isError } = useQuery(() => fetch("/api/data"))

const apiRef = useGridApiRef()
useEffect(() =>
  apiRef.current.subscribeEvent("stateChange", state => {
    localStorage.setItem("density", gridDensityValueSelector(state, apiRef.current.instanceId))
  })
, [apiRef.current])

if (isError) {
  return <p>Something went wrong</p>
}

return <DataGridPro
  apiRef={apiRef}
  density={localStorage.getItem("density") || "standard"}
  rows={data}
  // ...
/>

Your environment 🌎

  System:
    OS: Linux …
  Binaries:
    …
  Browsers:
    Chrome: Not Found
    Firefox: 106.0.5
  npmPackages:
    @emotion/react: ^11.10.5 => 11.10.5 
    @emotion/styled: ^11.10.5 => 11.10.5 
    @mui/base:  5.0.0-alpha.105 
    @mui/core-downloads-tracker:  5.10.13 
    @mui/icons-material: ^5.10.9 => 5.10.9 
    @mui/material: ^5.10.13 => 5.10.13 
    @mui/private-theming:  5.10.9 
    @mui/styled-engine:  5.10.8 
    @mui/system:  5.10.13 
    @mui/types:  7.2.0 
    @mui/utils:  5.10.6 
    @mui/x-data-grid:  5.17.11 
    @mui/x-data-grid-pro: ^5.17.11 => 5.17.11 
    @mui/x-date-pickers:  5.0.8 
    @mui/x-date-pickers-pro: ^5.0.8 => 5.0.8 
    @mui/x-license-pro:  5.17.11 
    @types/react: ^18.0.25 => 18.0.25 
    react: ^18.2.0 => 18.2.0 
    react-dom: ^18.2.0 => 18.2.0 
    typescript: ^4.8.4 => 4.8.4

Order ID 💳 (optional)

We have one but I have to check if I'm allowed to post it.

[m4theushw]

We don't set explicitly apiRef.current = null when unmounting. It's the useImperativeHandle hook that does, as the same happens with a ref during unmount.

https://github.com/mui/mui-x/blob/102cffc122c79b86a723185a5c0217c29c7af31d/packages/grid/x-data-grid/src/hooks/core/useGridApiInitialization.ts#L82

We can add null to the accepted values for apiRef.current. It will solve the warning from the prop-types.

diff --git a/packages/grid/x-data-grid-pro/src/models/dataGridProProps.ts b/packages/grid/x-data-grid-pro/src/models/dataGridProProps.ts
index c4a383678..669a9e3c3 100644
--- a/packages/grid/x-data-grid-pro/src/models/dataGridProProps.ts
+++ b/packages/grid/x-data-grid-pro/src/models/dataGridProProps.ts
@@ -132,7 +132,7 @@ export interface DataGridProPropsWithoutDefaultValue<R extends GridValidRowModel
   /**
    * The ref object that allows grid manipulation. Can be instantiated with [[useGridApiRef()]].
    */
-  apiRef?: React.MutableRefObject<GridApiPro>;
+  apiRef?: React.MutableRefObject<GridApiPro | null>;
   /**
    * The initial state of the DataGridPro.
    * The data in it will be set in the state on initialization but will not be controlled.
diff --git a/packages/grid/x-data-grid/src/models/props/DataGridProps.ts b/packages/grid/x-data-grid/src/models/props/DataGridProps.ts
index 79744a8ee..1ac29cd88 100644
--- a/packages/grid/x-data-grid/src/models/props/DataGridProps.ts
+++ b/packages/grid/x-data-grid/src/models/props/DataGridProps.ts
@@ -350,7 +350,7 @@ export interface DataGridPropsWithoutDefaultValue<R extends GridValidRowModel =
    * TODO: Remove `@internal` when opening `apiRef` to Community plan
    * @ignore - do not document.
    */
-  apiRef?: React.MutableRefObject<GridApiCommunity>;
+  apiRef?: React.MutableRefObject<GridApiCommunity | null>;
   /**
    * Signal to the underlying logic what version of the public component API
    * of the data grid is exposed [[GridSignature]].

Then we'll have to update a bunch of other files to also accept null.

As a workaround for now, you can move <DataGridPro> and useGridApiRef to a dedicated component, and render conditionally this component.

[joespeargresham]

Is this likely to get fixed please? Unfortunately we can't move our useGridApiRef call into a dedicated component because we need the apiRef higher up in the hierarchy.

[artola]

I fall into the same problem. Rendering conditionally might solve partially the problem but leads to others.

Look this construction, we have 2 components and we switch the view among them. We do not want to dismount them, otherwise we lose the state on all the components inside, thus we use "freeze".

  <div>
      <Freeze freeze={viewId !== 1}>
        <FooGrid data={foo_data} />
      </Freeze>
      <Freeze freeze={viewId === 1}>
        <BarGrid data={bar_data} />
      </Freeze>
  <div />

Switching from Foo to Bar works, when we return to Foo the warning is shown.

[jewseppi]

Is there a plan to resolve this? seems like an open issue for a while. I'm having the same problem with the pro version.

[scamden]

This also happens when the grid is Suspended. https://codesandbox.io/p/sandbox/datagrid-null-ref-on-suspense-72nnlt (edit: wrong link, updated)

Note: in the sandbox this only produces a warning but in our production deploy GridRow still renders even though the component is suspended (not sure how. working on a tighter repro) and causes null reference errors!

Is there any recommendation around using DataGrid with suspense?

cc @m4theushw @romgrk

[romgrk]

From what I read in the top comment, this isn't throwing an error, it's displaying a warning. If it's the case, the workaround listed there (setting the ref value to an empty object) is also the best way to deal with this issue. This is a prop-types runtime warning that is safe to ignore, and they should never be enabled on a production build.

I don't think that widening the typings to accept null is a good idea, because it would force every use of the ref to assert existence (apiRef.current!.xxxxx).

@scamden If there is a runtime error, provide the reproduction case and we'll take a look.

[scamden]

I don't think that widening the typings to accept null is a good idea, because it would force every use of the ref to assert existence (apiRef.current!.xxxxx).

@romgrk I would agree accept that the use of useImperativeHandle means that .current can in fact be null at times so making the type include null lines it up with reality. Unfortunately the workaround of setting to an empty object is even worse because then null checks need to be made on every api method. Treating current as null until it truly has all the methods is actually much safer.

I do have a runtime error. I'm having a hard time creating a repro in codesandbox but basically GridRow renders while a parent is suspending (and therefore current is null). Take a look at the code sandbox I provided which shows that the ref becomes null during suspension (which seems a lot more dangerous than during unmount and is the source of the run time error I haven't easily reproduced)

I have also seen apiRef.current be null during isGroupExpandedByDefault in the grid's initial setup phase fwiw.

[romgrk]

If you need clean conditional rendering, you can wrap the apiRef inside a wrapper component with the datagrid and re-export it with useImperativeHandle, so that on the outside you have a way to access the API, and in the inside the grid always has a clean API ref object: https://stackblitz.com/edit/mui-mui-x-2hvdmv?file=src%2Fdemo.tsx

The runtime errors are really worrying though, if you're able to get us a reproduction it would be greatly appreciated. The logic is supposed to initialized apiRef.current at render-time before any other code has a chance to touch the API, so it's not supposed to be null.

[scamden]

Here i've finally got a repro for this: click Suspend button and witness the errror: https://codesandbox.io/p/sandbox/datagrid-null-ref-on-suspense-72nnlt

[romgrk]

I think we can avoid the issue by avoiding useImperativeHandle and just using the ref'ed object directly and never setting the ref to null.

@mui/xgrid Any thoughts on dropping useImperativeHandle? Or the alternative, adding null to the useGridApiRef typing?

[cherniavskii]

we can avoid the issue by avoiding useImperativeHandle and just using the ref'ed object directly and never setting the ref to null.

This sounds good to me! This might break some tests, but I think we can make it work.

[romgrk]

I've thought about it some more, and not setting it to null might cause API calls to go through even though they shouldn't. Suspense unmounts the DOM elements, so maybe some operations could have weird results? I'm not sure. Maybe adding | null to the type could make more sense. It would probably break some user builds though. Wdyt?

[cherniavskii]

I created a minimal reproduction example using the DataGrid without row grouping. While the issue is reproducible in Data Grid v6, I can't reproduce it in v7.

v6: https://stackblitz.com/edit/react-1fe1ex?file=Demo.tsx v7: https://stackblitz.com/edit/react-1fe1ex-lszfms?file=Demo.tsx

I believe the issue was fixed by https://github.com/mui/mui-x/pull/11792

@scamden Can you give v7 a try?

[github-actions[bot]]

The issue has been inactive for 7 days and has been automatically closed.

[romgrk]

Did that PR fix the issue completely?

[github-actions[bot]]

The issue has been inactive for 7 days and has been automatically closed.

[sterlingdcs-damian]

Is there a good workaround for this? I would upgrade to the next major but the codemod doesnt work yet

[mparsakia]

We're running into the same issue. This is probably the most frustrating thing when using the datagrid ref. There is no reliable way to tell react when the const apiRef = useGridApiRef(); methods are truly ready for use.

This is a problem when trying to use apiRef methods inside useLayoutEffect, especially on first render when we need to setup a large amount of things to the grid. The hacky workaround is to use the not documented onStateChange prop onStateChange={(params, event, details) => {...}} to in combination with a boolean local state that checks various things like params?.dimensions?.isReady and details?.api?.rootElementRef?.current, and when those things are defined set it to true triggers the deps of the useEffect to run properly.

Are there any plans for a onReady or similar prop?