Replace rowGetter and rowsCount with rows[]

[nstepien]

I think rowGetter is an antipattern: if rowGetter never changes, then it will hide row updates, and might prevent rerenders in memoized components/useMemo.

The downside is that users will now need to load all the rows at once.
We're also updating rdg to leave value access to the formatter/editor implementations, so not all data needs to be computed upfront.
I wonder if we even need row objects at all! Formatters and editors could just load the row data with only the row index. But then how do we trigger cells rerendering?
Alternatively, users could use a Proxy on rows or as row objects. 🤔

WDYT?

[qili26]

@nstepien

The downside is that users will now need to load all the rows at once.

I don't think this is an issue as we can use endless scrolling to call API to pull the new data. #1854 If it's for memory consideration for huge data set, maybe the dev should just implement the pagination.

I wonder if we even need row objects at all! Formatters and editors could just load the row data with only the row index. But then how do we trigger cells rerendering?

If we pass the row idx / column idx only, then probably we have to discard the React.Memo in row / cell level inside RDG, so that they will always re-render. However, I guess that's not a feasible approach.

Alternatively, users could use a Proxy on rows or as row objects.

Looks like Proxy is not IE 11 friendly. However, just out of curiosity, could you extend a little bit like how you would use it?

[nstepien]

could you extend a little bit like how you would use it?

Something like that:

const rowsProxy = new Proxy({}, {
  get(obj, prop) {
    if (prop === 'length') return rowsCount;
    return rowGetter(Number(prop));
  }
});

return <DataGrid rows={rowsProxy} />;

[amanmahajan7]

LGTM. I think for lazy loading we can now just use the onScroll prop. Let's wait for @qili26 to review this PR as well

[amanmahajan7]

Nm, just realized he has already approved it

[ShantanuNair]

LGTM. I think for lazy loading we can now just use the onScroll prop. Let's wait for @qili26 to review this PR as well

Has the onScroll feature been merged into canary or pre-canary yet? #1854 hasn't been merged in yet as far as I'm aware.