mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.9k stars 1.18k forks source link

[data grid] Data Grid Pro scrollToIndexes Throws Error while testing using react-testing library #6411

Open saikiranraparthi opened 1 year ago

saikiranraparthi commented 1 year ago

Duplicates

Latest version

Current behavior 😯

Current Behaviour:

When I'm testing the component that uses DataGridPro using scrollToIndexes func throws the below error. I don't see this error in the app, probably because the function scrollToIndexes is called after loading the data and we get the data via AJAC call. As we mock the data in test, it is not working and probable because scrollToIndexes cannot be called soon?

I feel this is one way related to https://github.com/mui/mui-x/issues/5071 but we don't see the problem in the app as we are waiitng for long before calling the scrollToIndexes function

Error:

Cannot read properties of undefined (reading 'current')
TypeError: Cannot read properties of undefined (reading 'current')
    at Object.scrollToIndexes (/Users/saikiranraparthi/Projects/storyline/node_modules/@mui/x-data-grid/node/hooks/features/scroll/useGridScroll.js:120:64)
    at Object.apiRef.current.<computed> [as scrollToIndexes] (/Users/saikiranraparthi/Projects/storyline/node_modules/@mui/x-data-grid/node/hooks/utils/useGridApiMethod.js:26:84)
    at /Users/saikiranraparthi/Projects/storyline/apps/frontend/src/features/AddressExplorationPage/hooks.ts:134:26
    at commitHookEffectListMount (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:23150:26)
    at commitPassiveMountOnFiber (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:24931:11)
    at commitPassiveMountEffects_complete (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:24891:9)
    at commitPassiveMountEffects_begin (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:24878:7)
    at commitPassiveMountEffects (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:24866:3)
    at flushPassiveEffectsImpl (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:27039:3)
    at flushPassiveEffects (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:26984:14)
    at performSyncWorkOnRoot (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:26076:3)
    at flushSyncCallbacks (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:12042:22)
    at commitRootImpl (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:26959:3)
    at commitRoot (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:26682:5)
    at finishConcurrentRender (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:25981:9)
    at performConcurrentWorkOnRoot (/Users/saikiranraparthi/Projects/storyline/node_modules/react-dom/cjs/react-dom.development.js:25809:7)
    at flushActQueue (/Users/saikiranraparthi/Projects/storyline/node_modules/react/cjs/react.development.js:2667:24)
    at act (/Users/saikiranraparthi/Projects/storyline/node_modules/react/cjs/react.development.js:2582:11)
    at /Users/saikiranraparthi/Projects/storyline/node_modules/@testing-library/react/dist/act-compat.js:63:25
    at renderRoot (/Users/saikiranraparthi/Projects/storyline/node_modules/@testing-library/react/dist/pure.js:159:26)
    at render (/Users/saikiranraparthi/Projects/storyline/node_modules/@testing-library/react/dist/pure.js:246:10)
    at renderAndWait (/Users/saikiranraparthi/Projects/storyline/apps/frontend/src/features/testUtils.tsx:79:24)
    at renderWithEverything (/Users/saikiranraparthi/Projects/storyline/apps/frontend/src/features/testUtils.tsx:99:37)
    at Object.<anonymous> (/Users/saikiranraparthi/Projects/storyline/apps/frontend/src/features/AddressExplorationPage/AddressNetflowsTable.spec.tsx:163:44)
    at Promise.then.completed (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-circus/build/utils.js:333:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-circus/build/utils.js:259:10)
    at _callCircusHook (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-circus/build/run.js:240:40)
    at async _runTest (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-circus/build/run.js:202:5)
    at async _runTestsForDescribeBlock (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-circus/build/run.js:97:9)
    at async _runTestsForDescribeBlock (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-circus/build/run.js:91:9)
    at async _runTestsForDescribeBlock (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-circus/build/run.js:91:9)
    at async run (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-circus/build/run.js:31:3)
    at async runAndTransformResultsToJestFormat (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:135:21)
    at async jestAdapter (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:92:19)
    at async runTestInternal (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-runner/build/runTest.js:411:16)
    at async runTest (/Users/saikiranraparthi/Projects/storyline/node_modules/jest-runner/build/runTest.js:499:34)

Expected behavior 🤔

When calling the scrollToIndexes, it should work normally and need to scroll to Indexes as per the documentation in both App and the Unit tests.

Steps to reproduce 🕹

Link to live example: https://codesandbox.io/s/compassionate-kirch-bpxoz1?file=/src/App.tsx

Steps:

  1. Called the scrollToIndexes function in useEffect as in the above live example

Context 🔦

We are unable to use this functionality because of this issue. And this is blocking out feature readiness.

Your environment 🌎

npx @mui/envinfo ``` System: OS: macOS 12.5.1 Binaries: Node: 16.15.0 - ~/.nvm/versions/node/v16.15.0/bin/node Yarn: Not Found npm: 8.13.1 - ~/Projects/storyline/node_modules/.bin/npm Browsers: Chrome: 106.0.5249.103 Edge: Not Found Firefox: Not Found Safari: 15.6.1 npmPackages: @emotion/react: 11.10.4 => 11.10.4 @emotion/styled: 11.10.4 => 11.10.4 @mui/base: 5.0.0-alpha.84 @mui/icons-material: 5.8.3 => 5.8.3 @mui/lab: 5.0.0-alpha.85 => 5.0.0-alpha.85 @mui/material: 5.8.3 => 5.8.3 @mui/private-theming: 5.8.6 @mui/styled-engine: 5.8.0 @mui/system: 5.8.6 @mui/types: 7.1.4 @mui/utils: 5.10.3 @mui/x-data-grid: 5.17.1 @mui/x-data-grid-pro: 5.17.1 => 5.17.1 @mui/x-date-pickers: 5.0.0-alpha.1 @mui/x-license-pro: 5.17.0 @types/react: 18.0.20 => 18.0.20 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 styled-components: 5.3.5 => 5.3.5 typescript: 4.8.4 => 4.8.4 ```

Order ID 💳 (optional)

50436

saikiranraparthi commented 1 year ago

I think this is in someway related to ticket https://github.com/mui/mui-x/issues/4674 or https://github.com/mui/mui-x/issues/5071.

yaredtsy commented 1 year ago

@saikiranraparthi its because apiRef.current is not a valid dependency for useEffect. when it is changed it doesn't re-render the component. the only solution i found is to delay it.

 useEffect(() => {
    setTimeout(() => {
      apiRef.current.scrollToIndexes({ rowIndex: 10 });
    }, 0);
  }, [apiRef]);
yaredtsy commented 1 year ago

AG grid has onGridReady API, which lets you know when the grid is ready. I could not find a similar event in mui data-grid. it would be nice to have it.

saikiranraparthi commented 1 year ago

Thanks for the response @yaredtsy. Yes indeed it would be nice if we have onGridReady API. Using setTimeout seems like a hacky solution to me! is there anyway that we could check some condition before calling scrollToIndexes function? or probably have this incorporated/fixed in the scrollToIndexes function to trigger once the grid is ready.

yaredtsy commented 1 year ago

@DanailH is there a way to check if the grid is ready?

cherniavskii commented 1 year ago

Hey @saikiranraparthi Thanks for reporting this.

Indeed, scrollToIndexes in React.useEffect throws an error, because there's no apiRef.current.windowRef yet. We need to investigate how we can solve the issue.

yaredtsy commented 1 year ago

hi @cherniavskii , i have tried to solve this and come up with this.

return (
  <VirtualScrollerComponent
    ref={(node: HTMLDivElement) => {
      if (node) {
        apiRef.current.windowRef = { current: node };
        apiRef.current.publishEvent('gridReady');
      }
    }}
    style={style}
    disableVirtualization={isVirtualizationDisabled}
  />
);
useEffect(() => {
    return apiRef.current.subscribeEvent("gridReady", () => {
      apiRef.current.scrollToIndexes({ rowIndex: 6 });
    });
  }, [apiRef]);

Demo: https://codesandbox.io/s/agitated-sea-6kwgbv?file=/src/App.tsx:725-883 commit: https://github.com/yaredtsy/mui-x/pull/3/commits/cf36d326f136e93328addc39e37eaf08ee3d53d8

cherniavskii commented 1 year ago

In terms of DX, adding gridReady event won't really solve the issue on the user end, since it will require more code in user land for it to work. Instead, we can support a prop (e.g. scrollCoordinates) that will scroll whenever the coordinates change. It will use the gridReady (or whatever we call it) internally, so the user doesn't have to bother about it.

benjitastic commented 1 year ago

A scrollCoordinates prop would be great and solve the scroll jump/flicker I am currently experiencing when trying to scroll the user to specific coordinates when the grid first renders.

alanrubin commented 1 year ago

I'm also in need of that - same use case, scroll to a specific row when the data grid is first rendered. Now using a hack with setTimeout, not ideal to be honest.

lauri865 commented 1 year ago

Wanted to share the best workaround I have found (after many hours), which feels the least hacky and the most performant (i.e. no visible scroll jank on mount):

useEffect(() => {
    if (!apiRef?.current || !gridState.scroll?.top) return;

    const unsubscribe = apiRef.current.subscribeEvent(
      "scrollPositionChange",
      () => {
        apiRef.current.scroll(gridState.scroll);
        unsubscribe();
      }
    );
  }, []);

The variable gridState.scroll is set as api.getScrollPosition() on grid unmount.

The first "scrollPositionChange" event published is equivalent of the scrollable element being ready. Works with both virtualization on and off.

rhettl commented 1 year ago

I'm having this issue using the non-pro version and I cannot fix because useGridApiRef is only available in Pro.

omamazainab00 commented 11 months ago

@rhettl It is available in community version too, I checked in the docs.

nukemonk commented 3 weeks ago

Hi. I just wanted to share my (temporary) solution to this problem. For me using a setTimeout isn't always working. It fails about once every ±10 times the app loads. Also a timer with an arbitrary duration should never be a solution to any technical problem. I manage to bypass it using MutationObserver that will keep observing until the virtual scroller element is found.

   const safeScrollToIndexes = useCallback(
    (...params: Parameters<typeof apiRef.current.scrollToIndexes>) => {
      if (apiRef.current.rootElementRef?.current) {
        const getVirtualScrollerElement = () =>
          apiRef.current.rootElementRef?.current &&
          apiRef.current.rootElementRef.current.querySelector(
            `.${gridClasses.virtualScroller}`
          );

        if (!getVirtualScrollerElement()) {
          const mutationObserver = new MutationObserver((_, observer) => {
            if (getVirtualScrollerElement()) {
              apiRef.current.scrollToIndexes(...params);
              observer.disconnect();
            }
          });

          mutationObserver.observe(apiRef.current.rootElementRef.current, {
            childList: true,
            subtree: true,
          });
        } else {
          // Safe to scroll normally
          apiRef.current.scrollToIndexes(...params);
        }
      }
    },
    [apiRef]
  );