6pac
Fix for CSS Rendering Issue at large row counts
Describe the bug
Hi @ghiscoding ,
Hope you're doing well.
Wondering if you have a moment to take a quick look at a bug / suggested fix?
Problem: I recently encountered a CSS rendering issue at large counts:
See correct styles here for comparison:
Solution: I found that it relates to this line of code in appendRowHTML for slick.grid.ts:
const frozenRowOffset = this.getFrozenRowOffset(row);
const rowDiv = Utils.createDomElement('div', { className: `ui-widget-content ${rowCss}`, role: 'row', style: { top: `${this.getRowTop(row) - frozenRowOffset}px` } });
let rowDivR: HTMLElement | undefined;
divArrayL.push(rowDiv);
if (this.hasFrozenColumns()) {
// it has to be a deep copy otherwise we will have issues with pass by reference in js since
// attempting to add the same element to 2 different arrays will just move 1 item to the other array
rowDivR = rowDiv.cloneNode(true) as HTMLElement;
divArrayR.push(rowDivR);
}
The following fix resolves the issue, and the repo tests still seem ok:
const frozenRowOffset = this.getFrozenRowOffset(row);
const topValue = this.getRowTop(row) - frozenRowOffset; // Calculated top value
const rowDiv = Utils.createDomElement('div', {
className: `ui-widget-content ${rowCss}`,
role: 'row',
style: { transform: `translateY(${topValue}px)` } // Using transform instead of top
});
If this all looks ok to you, wondering if a special access is required to push a branch with the fix ? As I tried to but it said access denied.
Thanks, Pete
Reproduction
Grid with approx 10m rows with grid line styles . - > close to 10m it will encounter rendering issues.
Which Framework are you using?
React
Environment Info
System:
OS: Linux 6.8 Ubuntu 24.04 LTS 24.04 LTS (Noble Numbat)
CPU: (24) x64 12th Gen Intel(R) Core(TM) i9-12900KF
Memory: 51.57 GB / 62.58 GB
Container: Yes
Shell: 5.9 - /usr/bin/zsh
Binaries:
Node: 20.15.1 - ~/.nvm/versions/node/v20.15.1/bin/node
Yarn: 1.22.22 - ~/.nvm/versions/node/v20.15.1/bin/yarn
npm: 10.8.2 - ~/.nvm/versions/node/v20.15.1/bin/npm
Browsers:
Chrome: 126.0.6478.182
Validations
- [X] Follow our Code of Conduct
- [X] Read the Wikis.
- [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
- [X] Check that this is a concrete bug. For Q&A open a GitHub Discussion.
- [X] The provided reproduction is a minimal reproducible example of the bug.
This seems slightly related to issue #297
cc @6pac
So in your code, you seem to completely replace the top style with transform of an Y value? What happens to the next few lines for the frozen column with the clone node? You're not showing them in the bottom portion of the code change, so I'm unsure if you're removing them or what not!?
With this change however, this will most probably break all Cypress E2E tests because a few of these tests rely on the top style to exist and query against it for the test, for example because the SlickGrid Virtual Scroll can replace and shift rows in the DOM (not visually but in the DOM tree), I rely on the top to get whatever rows I want. What I mean is that if you filter some values in SlickGrid and you scroll down and up, you can't rely on let say give me 3rd row of querySelectorAll('.slick-row') because this might give you row 10 instead, however we can use top to get 3rd row (row height * 3 => top: 75px) which is convenient in a Cypress test and I use it often... so changing top to transform might have a lot of implications. I think this is a risky move, other users might also be using the same technique in their own Cypress tests too (or any other kind of testing or even in UI code, who knows), so this might be a Breaking Change (aka requiring a new major version), so it's not a small change
wondering if a special access is required to push a branch with the fix ? As I tried to but it said access denied.
That's the default behavior of GitHub and any other repos using git, no one in the world has access to push directly to a repo. You need to fork the repo and then push to your fork. After pushing to your fork, it will then ask you if you want to create a Pull Request to push to the original repo (here). So it's not a 1 step push, but rather a 3 steps push (unless you're a contributor/maintainer of this repo which are only 6pac and myself).
This Video explains how to contribute to an open source project: Your First GitHub Pull Request (in 10 minutes)
Hey there,
Great, thanks for looking into this so quickly.
Please find responses below:
- Frozen Column - this is the rest of the method:
protected appendRowHtml(divArrayL: HTMLElement[], divArrayR: HTMLElement[], row: number, range: CellViewportRange, dataLength: number) {
const d = this.getDataItem(row);
const dataLoading = row < dataLength && !d;
let rowCss = 'slick-row' +
(this.hasFrozenRows && row <= this._options.frozenRow! ? ' frozen' : '') +
(dataLoading ? ' loading' : '') +
(row === this.activeRow && this._options.showCellSelection ? ' active' : '') +
(row % 2 === 1 ? ' odd' : ' even');
if (!d) {
rowCss += ' ' + this._options.addNewRowCssClass;
}
const metadata = (this.data as CustomDataView<TData>)?.getItemMetadata?.(row);
if (metadata?.cssClasses) {
rowCss += ' ' + metadata.cssClasses;
}
const frozenRowOffset = this.getFrozenRowOffset(row);
const topValue = this.getRowTop(row) - frozenRowOffset; // Calculated top value
const rowDiv = Utils.createDomElement('div', {
className: `ui-widget-content ${rowCss}`,
role: 'row',
style: { transform: `translateY(${topValue}px)` } // Using transform instead of top
});
let rowDivR: HTMLElement | undefined;
divArrayL.push(rowDiv);
if (this.hasFrozenColumns()) {
rowDivR = rowDiv.cloneNode(true) as HTMLElement;
divArrayR.push(rowDivR);
}
let colspan: number | string;
let m: C;
for (let i = 0, ii = this.columns.length; i < ii; i++) {
m = this.columns[i];
if (!m || m.hidden) { continue; }
colspan = 1;
if (metadata?.columns) {
const columnData = metadata.columns[m.id] || metadata.columns[i];
colspan = columnData?.colspan || 1;
if (colspan === '*') {
colspan = ii - i;
}
}
// Do not render cells outside of the viewport.
if (this.columnPosRight[Math.min(ii - 1, i + (colspan as number) - 1)] > range.leftPx) {
if (!m.alwaysRenderColumn && this.columnPosLeft[i] > range.rightPx) {
// All columns to the right are outside the range.
break;
}
if (this.hasFrozenColumns() && (i > this._options.frozenColumn!)) {
this.appendCellHtml(rowDivR!, row, i, (colspan as number), d);
} else {
this.appendCellHtml(rowDiv, row, i, (colspan as number), d);
}
} else if (m.alwaysRenderColumn || (this.hasFrozenColumns() && i <= this._options.frozenColumn!)) {
this.appendCellHtml(rowDiv, row, i, (colspan as number), d);
}
if ((colspan as number) > 1) {
i += ((colspan as number) - 1);
}
}
}
- Cypress Tests I have ran the tests and can confirm that the Cypress tests do not pass (based on the issue mentioned - the grid itself seems to be functioning well). It sounds like whilst the fix seems straightforward that it throws off the tests? I'm not exactly sure what to say in response to this, as I probably don't know enough about the areas you've highlighted there to suggest anything helpful at this stage.
However, with regards to the bugfix, in terms of motivational sentiment I remember coming across documentation or possibly a github issue when first using Slickgrid about a year back, and it said that '2 billion rows are supported'. This made me very excited, as it was a big promise. I'm praying that can pull through, and at 100m so far and counting!.
- Github issues Brilliant, thanks for that! Super helpful. Happy to fork it and submit it for review?
Yes some people use it with extremely large dataset, I did see someone using that much data but I would never go with that much size myself (it's not recommended especially for sorting/grouping/...).
I already mentioned why the Cypress tests will fail, I'm often using the top with a querySelector to find the x row in the grid and now you want to remove top and replace it with transform, so yes the tests will for sure fail and potentially break a few user's usage. This really looks like a Breaking Change to me
I don't really want to use compare tools to find out what really changed, it would be helpful if you could use the ```diff instead and add + for new/change with lines of code and - for deleted/changed previous lines of code with, for example
- hello world
+ Hello World!
I'm not too sure that I want to proceed considering that this is most likely a Breaking Change (avoiding breaking changes would be ideal).
Look I'm just trying to help and be responsive, by contributing what I believe is a code fix to a genuine bug that I encountered when using the library. Above, I've run the tests in response to the comments.
Please find the requested diff details in the attached .diff zipFile changes-diff.zip.
That's completely understandable if it breaks a lot of tests and creates a lot of work. I thought I'd take the time to raise it, so others could benefit from the fix.
Thanks
-
- }
+ let rowDivR: HTMLElement | undefined;
+ divArrayL.push(rowDiv);
+ if (this.hasFrozenColumns()) {
+ rowDivR = rowDiv.cloneNode(true) as HTMLElement;
+ divArrayR.push(rowDivR);
}
- // Do not render cells outside of the viewport.
- if (this.columnPosRight[Math.min(ii - 1, i + (colspan as number) - 1)] > range.leftPx) {
- if (!m.alwaysRenderColumn && this.columnPosLeft[i] > range.rightPx) {
- // All columns to the right are outside the range.
- break;
+ let colspan: number | string;
+ let m: C;
+ for (let i = 0, ii = this.columns.length; i < ii; i++) {
+ m = this.columns[i];
+ if (!m || m.hidden) { continue; }
+
+ colspan = 1;
+ if (metadata?.columns) {
+ const columnData = metadata.columns[m.id] || metadata.columns[i];
+ colspan = columnData?.colspan || 1;
+ if (colspan === '*') {
+ colspan = ii - i;
+ }
}
- if (this.hasFrozenColumns() && (i > this._options.frozenColumn!)) {
- this.appendCellHtml(rowDivR!, row, i, (colspan as number), d);
- } else {
+ // Do not render cells outside of the viewport.
+ if (this.columnPosRight[Math.min(ii - 1, i + (colspan as number) - 1)] > range.leftPx) {
+ if (!m.alwaysRenderColumn && this.columnPosLeft[i] > range.rightPx) {
+ // All columns to the right are outside the range.
+ break;
+ }
+
+ if (this.hasFrozenColumns() && (i > this._options.frozenColumn!)) {
+ this.appendCellHtml(rowDivR!, row, i, (colspan as number), d);
+ } else {
+ this.appendCellHtml(rowDiv, row, i, (colspan as number), d);
+ }
+ } else if (m.alwaysRenderColumn || (this.hasFrozenColumns() && i <= this._options.frozenColumn!)) {
this.appendCellHtml(rowDiv, row, i, (colspan as number), d);
}
- } else if (m.alwaysRenderColumn || (this.hasFrozenColumns() && i <= this._options.frozenColumn!)) {
- this.appendCellHtml(rowDiv, row, i, (colspan as number), d);
- }
- if ((colspan as number) > 1) {
- i += ((colspan as number) - 1);
+ if ((colspan as number) > 1) {
+ i += ((colspan as number) - 1);
+ }
}
- }
}
protected appendCellHtml(divRow: HTMLElement, row: number, cell: number, colspan: number, item: TData) {
Yeah sure I'm open to progressive comments and discussions, what I meant to say is that I just think that we have to consider the potential side effect in doing this kind of changes because that could impact end users which is something that I always try to avoid (even if it fixes a bug, it might still be a Breaking Change because of the behavior change). Taking a look at your new code, it's more than I expected, have you tested frozen grids and all? The Cypress tests would eventually for sure all have to pass (probably by just changing top query selector to transform)
For example with the Cypress test shown below, it ask for top: ${GRID_ROW_HEIGHT * 0}px; which is basically the 1st grid row, then * 1 is 2n row, and so on and so on. I use this approach because of the SlickGrid Virtual Scroll that often changes the div orders in the DOM, the only constant is the top which is how it works in the UI and is good to use in the Cypress tests
https://github.com/6pac/SlickGrid/blob/87b8c6af4bed91bb646971b96d81185f346e7d2c/cypress/e2e/example-autotooltips.cy.ts#L39-L41
So in the end, I didn't mean that I will refuse the PR, but I meant to say is that if it's really the only way to go, then that would potentially (probably) require a major version v6.0 because of the Breaking Change behavior (changing from top to transform) and that is because of the potential side effect that could break user's code or expectations. However doing a major version for this small change seems a little too much... so my question is, is it really the only way to fix this issue? isn't there a way that we can keep the top somehow and still fix the issue without requiring a major version? If it's really the only way, then I find the list of changes for a major version to be very light.
I wonder why a transform is working well but top isn't always working as well? Do you have articles about this kind of change and issues? How did you come up with such a solution?
Sure, no problem, I appreciate that. Thanks.
Ok, so with regards to translate vs top, I’d been reading a book on front end which suggested that translate gets put on the GPU and accelerates paint times, compared to top which (generally) has longer paint times. The video at this link gets to the crux at 7:48 https://www.paulirish.com/2012/why-moving-elements-with-translate-is-better-than-posabs-topleft/ , and makes a convincing argument using browser tools to measure paint times. Part of me is wondering whether this is still the case in 2024 (?), but it’s interesting for sure. Also, apparently translate handles larger numbers better, which is why I initially went to it as assumed I’d hit some weird overflow issues due to the number sizes.
With regards to the versioning, I understand your concerns. For a personal project, I’m at the point where I need to make a call on Slickgrid, and it’s been very promising in most areas. However a related area that’s causing a few issues is the repainting times, affecting the frame rate after fast scrolling. It’s still very fast data though not 45-60 FPS fast enough that casual users might expect from a spreadsheet-like tool, in that when holding down scroll (medium speed - not the slow arrow scroll, or moving the bar, but the scroll bar space between) the data and grid doesn’t repaint in time for it to look like the data is being scrolled through. It does manage to get 30% of the data in time though (which has the effect of looking like the data is chasing the grid). I should note that this is compounded by the way data is brought in for my specific use case and, but that has already been heavily optimised and therefore I believe any achievable optimisations on the library code could still have potential to benefit the general library.
A related UX drawback I’ve experienced is the white flashing after scrolling which as I understand from one of your stack overflow posts is part of the virtual scrolling, but Ive wondered if there is still maybe some opportunity here? Perhaps not? But it’s still a different UX to say Excel/google sheets, and I felt concerned that a user might get disoriented. Not for jumping from say row 1000 to 30,000, but for scrolling between adjacent numbers (at high speed) I feel it would be awesome if it could be a bit smoother.
Why am I raising this? Well if it’s is ok with you I’d like to have an go at diving deeper on the performance side to see if I can profile it, review the internals in more detail and potentially recommend further adjustments?
Can’t promise it will bear any fruit, as I know you guys have worked with this for a long time, but if I were to break any ground (rather than tests 😅) and bundle a few related changes up, try pass the tests, then would you be happy to take a look at it ? I would have no issue if it sat on the back burner until it made sense to bundle as part of a broader release, or otherwise I can always just run my own patch which I’ve done for a couple of small things, which would be fine. Anyway, keen to have a crack and see if it’s possible to speed up the fast-scroll rendering, if this ok?
Note - would expect up to a 4-8 week turnaround with other commitments, getting familiar with the library and some of the front end optimisations, depending on how difficult it is. Just to help set expectations.
Thanks
Note I should have mentioned if it’s a matter of porting tests I’m happy to check that out at the same time.
Edit: Also noting that the above includes tweaking the library debounce setting from 50ms to 5ms, which helped the data appear partially on the grid during scroll.
From my point of view, the issues you raise are very valid and I'd definitely support working on them.
It probably remains to be seen what the impact would be on the codebase, but if there were very tangible visual improvements, then I'd say it would be worth some degree of pain to implement them.
I'll let @ghiscoding speak for himself, but I suspect he'll say the same thing.
There's a throttle in place which makes the virtual scroll wait before rendering the next batch. It's currently set to 50ms, you can try to decrease it. I think it was added by Ben a long time ago (it doesn't seem to exist in the original SlickGrid repo though). You have a large dataset so you are the best person who could give it a try with maybe 0 and see what happens (there's more info in old PR #810).
https://github.com/6pac/SlickGrid/blob/87b8c6af4bed91bb646971b96d81185f346e7d2c/src/slick.grid.ts#L290
I actually always wondered why that was put in place, so perhaps you could try to disable it completely, something like this maybe. So if you comment out the following line (and where it's being used, which is the enqueue and dequeue). There's a good chance that it would make the rendering experience worst which would help explain why this code was put in place, but without really giving it a try on a large dataset like yours, it's hard to know for sure without testing it.
https://github.com/6pac/SlickGrid/blob/87b8c6af4bed91bb646971b96d81185f346e7d2c/src/slick.grid.ts#L663
https://github.com/6pac/SlickGrid/blob/87b8c6af4bed91bb646971b96d81185f346e7d2c/src/slick.grid.ts#L4918
https://github.com/6pac/SlickGrid/blob/87b8c6af4bed91bb646971b96d81185f346e7d2c/src/slick.grid.ts#L5105
there's also a grid option for how many rows to keep in the buffer when scrolling, which in other words mean how many row do we want to cache and make ready before the scroll happens. I'm not sure if it has any impacts or not but maybe try to increase it a bit?
https://github.com/6pac/SlickGrid/blob/87b8c6af4bed91bb646971b96d81185f346e7d2c/src/slick.grid.ts#L283
Note I should have mentioned if it’s a matter of porting tests I’m happy to check that out at the same time.
it would be part of it, but the question is more about "Do we really want to do a Breaking Change version with just this change? or can we do a bug fix that will not introduce a Breaking Change". Because in my head, I'm really consider the top to transform to be a Breaking Change and I'm trying hard to not push a new major yet again, but if it's really the only solution that works then yeah we'll go ahead with it.
Doing some tests on a large dataset with the area that I mentioned above might be where the perf can be improved. I always wondered what side effect it could have to go without this code. I think there's probably a good (better) number to come up with that will make it look more fluid while not making worst either. I mean without the render throttle, it might make it look worst because the browser can't keep up, but again we won't know until someone is performing local tests of that. Browser are getting better every year so the throttle we needed 5 years ago might not be the same or as worth it today (it's also probably a good idea to test with more than 1 browser).
So anyway, doing more tests and thinking about what else we could include in a Breaking Change version. I mean, if there are other breaking changes, now would be the good time
not aware of ever having added the throttle, but if you have evidence, then I suppose it was me! Way back when this was happening, there was much less standardisation between browsers so it could well have been a cross-browser compromise. I think in the modern context we would be much more confident in the stability of a more optimal solution.
not aware of ever having added the throttle, but if you have evidence, then I suppose it was me! Way back when this was happening, there was much less standardisation between browsers so it could well have been a cross-browser compromise. I think in the modern context we would be much more confident in the stability of a more optimal solution.
haha yeah sorry it's not about pointing fingers, but more about the reason why it was put in place. I suspect that might have been copied from another fork too, there was a bunch of different forks that existed when the original got unsupported, so who knows where and why this code was put in place is hard to remember ...
anyway, you can see similar discussions we had about this a while ago in this comment and the commit where you added this throttling is 93f043fa
So I did. I'm pretty sure I didn't write that code though - it must have been either patched in from another repo or contributed in an issue. So in summary, I don't know exactly why it was added. It does definitely make sense to have some throttling for long scrolls (that was probably my logic at the time), but there appears to be some room for improvement.
Great. Thanks for your comments too @6pac. Well this sounds like a great place to start @ghiscoding - I’ll run those preliminary tests over the weekend, and post back with some benchmark comparisons on the parameters listed. If I learn anything else when diving I’ll include it. If there’s anything else specific like that just let me know . Cheers.
anyway, you can see similar discussions we had about this a while ago in this comment and the commit where you added this throttling is 93f043f
@pbower there's a bit more info in that comment (and thread) that I referenced, it seems like 5ms was a good number to use when I tested it at that time.
ohhh wait a second, I just found how to push this into a new version without making a Breaking Change. We could simply add this into a grid option keep top as the default and that's it. You'll be able to change it to a transform while not breaking other expectations and everyone is happy.. woohoo! 🎉 something like this in the GridOption interface topStyleRenderType: 'top' | 'transform' (I'm not too crazy about the name, I'm sure you can find better) would be the default and then just use either or in the code...
const rowDiv = Utils.createDomElement('div', {
className: `ui-widget-content ${rowCss}`,
role: 'row',
- style: { transform: `translateY(${topValue}px)` } // Using transform instead of top
});
+ if (options.topStyleRenderType === 'transform') {
+ rowDiv.style.transform = `translateY(${topValue}px)`;
+ } else {
+ // else default to `top`
+ rowDiv.style.top = `${topValue}px`;
+ }
and voila! Perf improvements is now doable on your side and it won't break users who are still expecting top because that would remain the default. If we ever decide to do a major version in the future, if it ever happen, then we could simply switch the default to transform if we wanted.
Also for the other portion of code that I reference above (scrollThrottle), it always wraps it in a setTimeout but perhaps what we could do is if the value is 0 then don't use the setTimeout neither the throttle callback but when the value is over 0 then use it the way it currently is (that code should be easy enough to change without breaking anything)
EDIT
I also see this comment in the article you referenced, which was very instructive. I think we are already using an integer for the top value but if not then please make sure it is (with Math.ceil() probably)
@pbower might be woth checking this out too: https://www.thecandidstartup.org/2024/04/29/modern-react-virtual-scroll-grid-9.html
Awesome, thanks for that. Just reaching out with an update - I did have some good success on the weekend both from a profiling and then action-ing perspective. I made a few key updates across :
Apply Html Code Append Cell Html Append Row Html Reorder Rows Render
These changes focused on batching DOM updates.
However, I then a roadblock when I went to run the tests, and unfortunately knocked out a few of the key event listeners in the process. So I will go back and QA it properly next weekend when I get some time and do that before sharing back the code and comparisons, otherwise it's kind of cheating I think.
Thanks
Really curious that repo there - I ran the infinigrid examples - 1 follower / star repo (i.e., like I was the first one to visit it), casually with a 3 trillion row grid sitting there. Seems like he's on a mission.
Yep. From what I can tell, former rockstar programmer gets tired of the stress, makes a well compensated exit and has a lot of time on his hands. My dream.
@pbower we didn't hear from your side, so I went ahead and done couple of small PRs related to this issue
- added a new grid option
rowTopOffsetRenderType?: 'top' | 'transform', so that you can start using it without breaking other users since the default remainstop(see PR #1050)- Note: I modified the Example - Grouping (ESM) to use the
transform, so you can also see it live) https://github.com/6pac/SlickGrid/blob/dc08ab801ccd69b4875bcaf22b2c36a80fc192b7/src/slick.grid.ts#L3950-L3957
- Note: I modified the Example - Grouping (ESM) to use the
- decreased
scrollRenderThrottlinggrid option to 10ms instead of previously 50ms, this should reduce the time it shows a blank section when scrolling to some rows that are uncached (for example scroll completely to the bottom of the grid) - I also found that
forceSyncScrollinggrid option (defaults to false) exists to completely bypass thescrollRenderThrottling
https://github.com/6pac/SlickGrid/blob/dc08ab801ccd69b4875bcaf22b2c36a80fc192b7/src/slick.grid.ts#L5113-L5118
This is all available in v5.12.0
Hey, sorry it looks I never came back to you on this. Thanks heaps for that, it's much appreciated. I will still dive into this when I get a chance - as it's something that I need a few days straight to dive into to do it properly.
@pbower Side note on the grid option that I added rowTopOffsetRenderType?: 'top' | 'transform', it actually looks like Ag-Grid is also using transform for perf reason. I found that out lately when I added the recent rowspan feature, taking a look at Ag-Grid, they had this note on Row Span
Ag-Grid - Row Spanning Configuring Row Spanning To allow row spanning, the grid must have property suppressRowTransform=true. Row spanning is then configured at the column definition level. To have a cell span more than one row, return how many rows to span in the callback colDef.rowSpan.
Their note is also true for our rowspan as well, it doesn't work well with transform and we need to use top for that feature, but for the rest using transform seems to have better perf as per what Ag-Grid is mentioning above.
So anyway, it's worth knowing about it 😃
@pbower have you tried the new grid option I added yet rowTopOffsetRenderType: 'transform'?
Hi @ghiscoding,
No worries at all, and thanks for reaching back out. I’m actually up in the Himalayas at the moment, a little off the "Grid" (pun intended!), but always happy to chat.
Great to see the reconnaissance mission in full effect. I’ve been using transform as the default, and it’s been working well—interesting to hear that the winner can shift depending on use case.
I realise I never properly followed up on those performance investigations, but did some deeper profiling and found a few key methods that consistently surfaced as rendering bottlenecks in Google Profiler, especially around DOM redraws:
- appendRowHtml
- appendCellHtml
- applyHtmlCode
- render
- renderRows
I experimented with batching updates and saw some minor gains, but I suspect we’re at the limits of what can be optimised purely within the JS layer. What do you think ? Have you explored much with these methods in the past?
That got me thinking—what if grid lines and background colours were handled via WebGL/WebGPU (e.g., PixiJS) instead of CSS? I wonder how much that could improve rendering speeds, especially for high-scroll, infinite canvas-style use cases.
To give a sense of what's possible, check out how these guys approach rendering pipelines (though their code is source available, closed-source, so more for inspiration): Infinite Canvas.
Would love to hear your thoughts on this. Is there any appetite for experimenting in that direction, even if just as an optional rendering mode? Or do you think the sweet spot still lies in optimising the existing approach?
Still hitting a few rendering bottlenecks even after optimising server code, so keen to see where this leads.
Cheers, Pete
Hey @ghiscoding and @6pac,
Hope you're both doing well. Just following up on this one again—I finally carved out the time to dive back into the gauntlet yesterday for a fresh look, and I’m pleased to say it looks like it's pretty close.
Here’s a side-by-side comparison of the new (WIP) version (left) versus the current version (right).
After isolating the problem with some refactoring yesterday, I really honed in on the five main methods above and tackled the bottlenecks. The biggest issues in rendering speed look like they stem from repeated DOM updates, which perform well enough on a smaller number of cells but start causing noticeable lag at higher cell counts and zoom levels.
So, here’s a fresh take on those five methods, optimised based on the following :
Minimising Direct DOM Manipulations – Instead of updating the DOM element by element, changes are batched and inserted in a single operation (using techniques like DocumentFragment). This reduces layout reflows and repaints.
Conditional Updates – The code now checks whether a new value is different from the current one before updating (e.g., comparing innerHTML or textContent before assignment). This prevents unnecessary DOM updates, reducing costly reflows.
Caching and Reusing Computed Values – Frequently used values (such as frozen row offsets, column positions, and computed CSS classes) are calculated once and reused, eliminating redundant calculations.
Efficient Cloning – In cases where a row needs to be rendered into multiple canvases (e.g., frozen columns), the new approach clones existing nodes instead of recreating them from scratch. This reduces processing overhead and ensures consistency.
Batching Render Operations – Rows and cells are collected in arrays and only appended in batches, reducing the number of individual DOM insertions. This is particularly beneficial when rendering a large number of elements.
After further testing, I’m feeling confident that these changes mitigate expensive layout operations, reduce repaints/reflows, and have the potential to improve the rendering pipeline’s efficiency.
However, it's unfortunately failing on a couple of last tests on the 'Example - Row Detail/Row Move/Checkbox Selector Plugins' test (see bottom stack trace below).
Wondering if you guys have any thoughts on this one ? Given it's a key part of the pipeline I'd find a second set of eyes on the below really helpful either way.
Additional Notes: (for the general case 515 tests that are passing
- Scrolling on large grids (e.g., 50 visible columns) feels much smoother. The difference is especially noticeable in high-density grids.
- The video comparison shows a 1ms setting on the real-time grid fast scroll, the old version intermittently hangs, while the WIP doesn't falter.
- Additionally, in the 500,000-row Dataview example (not in the video), there's no drastic visible difference, but side-by-side testing revealed that the old version occasionally "hangs" for a few milliseconds, whereas the new one doesn't stall.
Keen to hear your thoughts.
Cheers, Pete
Updated functions (all in slick.grid.ts):
/**
* Apply HTML code by 3 different ways depending on what is provided as input and what options are enabled.
* 1. If value is an HTMLElement or DocumentFragment, first empty the target and then append it.
* 2. If value is string/number/boolean and `enableHtmlRendering` is enabled, use target.innerHTML.
* 3. Otherwise, use target.textContent.
* @param {HTMLElement} target - Target element to apply to.
* @param {string | number | boolean | HTMLElement | DocumentFragment} val - Input value.
* @param {{ emptyTarget?: boolean; skipEmptyReassignment?: boolean; }} [options] -
* - emptyTarget (default true): whether to empty the target first.
* - skipEmptyReassignment (default true): if enabled, do not reassign empty content.
*/
applyHtmlCode(
target: HTMLElement,
val: string | number | boolean | HTMLElement | DocumentFragment,
options?: { emptyTarget?: boolean; skipEmptyReassignment?: boolean; }
) {
if (!target) {
return;
}
const { emptyTarget = true, skipEmptyReassignment = true } = options || {};
if (val instanceof HTMLElement || val instanceof DocumentFragment) {
// first empty target and then append new HTML element
if (emptyTarget) {
Utils.emptyElement(target);
}
target.appendChild(val);
} else {
// When already empty and reassigning empty, skip assignment.
if (skipEmptyReassignment && (!Utils.isDefined(val) || val === '') && !target.innerHTML) {
return;
}
// Since val is not an HTMLElement/DocumentFragment, it must be string|number|boolean.
let sanitizedText: string | number | boolean = val;
if (typeof sanitizedText === 'number' || typeof sanitizedText === 'boolean') {
const textVal = sanitizedText.toString();
if (target.textContent !== textVal) {
target.textContent = textVal;
}
} else {
// sanitizedText is a string here.
sanitizedText = this.sanitizeHtmlString(val as string);
if (this._options.enableHtmlRendering && sanitizedText) {
if (target.innerHTML !== sanitizedText) {
target.innerHTML = sanitizedText;
}
} else {
if (target.textContent !== sanitizedText) {
target.textContent = sanitizedText;
}
}
}
}
}
/**
* Iterates over the row indices in the given rendered range and, for each row not yet in the cache,
* creates a new cache entry and calls appendRowHtml to build the row’s cell content. Once built,
* the row is appended to the appropriate canvas element (top or bottom, left or right depending on frozen settings).
* If the active cell is rendered, it reselects it.
*
* @param {{ top: number; bottom: number; leftPx: number; rightPx: number; }} range - The range of rows to render.
*/
protected renderRows(range: { top: number; bottom: number; leftPx: number; rightPx: number; }) {
const divArrayL: HTMLElement[] = [];
const divArrayR: HTMLElement[] = [];
const rows: number[] = [];
let needToReselectCell = false;
const dataLength = this.getDataLength();
const mustRenderRows = new Set<number>();
const renderingRows = new Set<number>();
for (let i = range.top; i <= range.bottom; i++) {
if (this.rowsCache[i] || (this.hasFrozenRows && this._options.frozenBottom && i === this.getDataLength())) {
continue;
}
this.renderedRows++;
rows.push(i);
renderingRows.add(i);
// Create an entry immediately so that appendRowHtml() can populate it.
this.rowsCache[i] = this.createEmptyCachingRow();
// Add rows intersecting with rowspan if applicable
if (this._options.enableCellRowSpan) {
const parentRowSpan = this.getRowSpanIntersect(i);
if (parentRowSpan !== null) {
renderingRows.add(parentRowSpan);
}
}
this.appendRowHtml(divArrayL, divArrayR, i, range, dataLength);
mustRenderRows.add(i);
if (this.activeCellNode && this.activeRow === i) {
needToReselectCell = true;
}
this.counter_rows_rendered++;
}
// Process any additional rows due to rowspan intersections.
const mandatorySpanRows = this.setDifference(renderingRows, mustRenderRows);
if (mandatorySpanRows.size > 0) {
mandatorySpanRows.forEach(r => {
this.removeRowFromCache(r); // Avoid duplicate DOM elements
rows.push(r);
this.rowsCache[r] = this.createEmptyCachingRow();
this.appendRowHtml(divArrayL, divArrayR, r, range, dataLength);
});
}
if (rows.length === 0) {
return;
}
// Batch DOM insertion using document fragments
const containerL = document.createDocumentFragment();
const containerR = document.createDocumentFragment();
divArrayL.forEach(elm => containerL.appendChild(elm));
divArrayR.forEach(elm => containerR.appendChild(elm));
// Append rows from the document fragments to the respective canvas elements.
rows.forEach(row => {
if (this.hasFrozenRows && row >= this.actualFrozenRow) {
if (this.hasFrozenColumns()) {
if (this.rowsCache[row] && containerL.firstChild && containerR.firstChild) {
this.rowsCache[row].rowNode = [containerL.firstChild as HTMLElement, containerR.firstChild as HTMLElement];
this._canvasBottomL.appendChild(containerL.firstChild as ChildNode);
this._canvasBottomR.appendChild(containerR.firstChild as ChildNode);
}
} else {
if (this.rowsCache[row] && containerL.firstChild) {
this.rowsCache[row].rowNode = [containerL.firstChild as HTMLElement];
this._canvasBottomL.appendChild(containerL.firstChild as ChildNode);
}
}
} else if (this.hasFrozenColumns()) {
if (this.rowsCache[row] && containerL.firstChild && containerR.firstChild) {
this.rowsCache[row].rowNode = [containerL.firstChild as HTMLElement, containerR.firstChild as HTMLElement];
this._canvasTopL.appendChild(containerL.firstChild as ChildNode);
this._canvasTopR.appendChild(containerR.firstChild as ChildNode);
}
} else {
if (this.rowsCache[row] && containerL.firstChild) {
this.rowsCache[row].rowNode = [containerL.firstChild as HTMLElement];
this._canvasTopL.appendChild(containerL.firstChild as ChildNode);
}
}
});
if (needToReselectCell) {
this.activeCellNode = this.getCellNode(this.activeRow, this.activeCell);
}
}
/**
* (re)Render the grid
*
* Main rendering method that first dequeues any pending scroll throttling, then obtains the visible and rendered ranges.
* It removes rows no longer visible, calls cleanUpAndRenderCells and renderRows to render missing cells and new rows,
* and, if frozen rows are present, renders them separately. It then sets post–processing boundaries, starts post–processing,
* updates scroll positions, and triggers the onRendered event.
*/
render() {
if (!this.initialized) {
return;
}
this.scrollThrottle.dequeue();
const visible = this.getVisibleRange();
const rendered = this.getRenderedRange();
// Remove rows no longer in the viewport
this.cleanupRows(rendered);
// Update cells if horizontal scroll has changed
if (this.lastRenderedScrollLeft !== this.scrollLeft) {
if (this.hasFrozenRows) {
const renderedFrozenRows = { ...rendered };
if (this._options.frozenBottom) {
renderedFrozenRows.top = this.actualFrozenRow;
renderedFrozenRows.bottom = this.getDataLength();
} else {
renderedFrozenRows.top = 0;
renderedFrozenRows.bottom = this._options.frozenRow!;
}
this.cleanUpAndRenderCells(renderedFrozenRows);
}
this.cleanUpAndRenderCells(rendered);
}
// Render missing rows
this.renderRows(rendered);
// Render frozen rows separately if applicable
if (this.hasFrozenRows) {
if (this._options.frozenBottom) {
this.renderRows({
top: this.actualFrozenRow,
bottom: this.getDataLength() - 1,
leftPx: rendered.leftPx,
rightPx: rendered.rightPx
});
} else {
this.renderRows({
top: 0,
bottom: this._options.frozenRow! - 1,
leftPx: rendered.leftPx,
rightPx: rendered.rightPx
});
}
}
this.postProcessFromRow = visible.top;
this.postProcessToRow = Math.min(this.getDataLengthIncludingAddNew() - 1, visible.bottom);
this.startPostProcessing();
this.lastRenderedScrollTop = this.scrollTop;
this.lastRenderedScrollLeft = this.scrollLeft;
this.trigger(this.onRendered, { startRow: visible.top, endRow: visible.bottom, grid: this });
}
/**
* Creates a row container element with CSS classes (e.g. active, odd/even, frozen, loading) based on the row’s state
* and metadata. It positions the row using getRowTop (adjusting for frozen rows), clones the row for frozen-column
* support if needed, and iterates over each column to call appendCellHtml
* for each cell that is within the visible viewport range.
*
* @param {HTMLElement[]} divArrayL - Array to store left–side row elements.
* @param {HTMLElement[]} divArrayR - Array to store right–side row elements (for frozen columns).
* @param {number} row - The row index to render.
* @param {CellViewportRange} range - The visible viewport range.
* @param {number} dataLength - Total data length to determine if the row is loading.
*/
protected appendRowHtml(
divArrayL: HTMLElement[],
divArrayR: HTMLElement[],
row: number,
range: CellViewportRange,
dataLength: number
) {
const d = this.getDataItem(row);
const dataLoading = row < dataLength && !d;
let rowCss = 'slick-row' +
(this.hasFrozenRows && row <= this._options.frozenRow! ? ' frozen' : '') +
(dataLoading ? ' loading' : '') +
(row === this.activeRow && this._options.showCellSelection ? ' active' : '') +
(row % 2 === 1 ? ' odd' : ' even');
if (!d) {
rowCss += ` ${this._options.addNewRowCssClass}`;
}
const metadata = this.getItemMetadaWhenExists(row);
if (metadata?.cssClasses) {
rowCss += ` ${metadata.cssClasses}`;
}
const rowDiv = Utils.createDomElement('div', {
className: `ui-widget-content ${rowCss}`,
role: 'row',
dataset: { row: `${row}` },
});
const frozenRowOffset = this.getFrozenRowOffset(row);
const topOffset = this.getRowTop(row) - frozenRowOffset;
if (this._options.rowTopOffsetRenderType === 'transform') {
rowDiv.style.transform = `translateY(${topOffset}px)`;
} else {
rowDiv.style.top = `${topOffset}px`; // default to `top: {offset}px`
}
divArrayL.push(rowDiv);
let rowDivR: HTMLElement | undefined;
if (this.hasFrozenColumns()) {
// Clone the row so that the same element is not moved between canvases.
rowDivR = rowDiv.cloneNode(true) as HTMLElement;
divArrayR.push(rowDivR);
}
const columnCount = this.columns.length;
for (let i = 0; i < columnCount; i++) {
const m = this.columns[i];
if (!m || m.hidden) {
continue;
}
// Set default colspan and rowspan as numbers.
let colspan = 1;
let rowspan = 1;
let columnData: ColumnMetadata | null = null;
if (metadata?.columns) {
columnData = metadata.columns[m.id] || metadata.columns[i];
if (columnData) {
// For colspan: if defined as a number use it; if equal to '*' then use remaining columns.
if (typeof columnData.colspan === 'number') {
colspan = columnData.colspan;
} else if (columnData.colspan === '*') {
colspan = columnCount - i;
}
// For rowspan: if defined as a number use it.
if (typeof columnData.rowspan === 'number') {
rowspan = columnData.rowspan;
}
}
// Ensure rowspan does not exceed available rows.
if (rowspan > dataLength - row) {
rowspan = dataLength - row;
}
}
// Don't render a child cell of a rowspan cell.
if (this.getParentRowSpanByCell(row, i)) {
continue;
}
if (this.columnPosRight[Math.min(columnCount - 1, i + colspan - 1)] > range.leftPx) {
let isRenderCell = true;
if (!m.alwaysRenderColumn && this.columnPosLeft[i] > range.rightPx) {
isRenderCell = false;
}
if (isRenderCell) {
const targetRowDiv = (this.hasFrozenColumns() && (i > this._options.frozenColumn!)
? rowDivR! : rowDiv);
this.appendCellHtml(targetRowDiv, row, i, colspan, rowspan, columnData, d);
}
} else if (m.alwaysRenderColumn || (this.hasFrozenColumns() && i <= this._options.frozenColumn!)) {
this.appendCellHtml(rowDiv, row, i, colspan, rowspan, columnData, d);
}
if (colspan > 1) {
i += (colspan - 1);
}
}
}
/**
* Creates a cell element with appropriate CSS classes (including frozen and active classes) and retrieves its value
* via the formatter. It applies additional CSS classes from event return values and formatter results,
* sets tooltips if provided, and inserts any additional DOM elements if required. It then appends the cell element
* to the row container and updates the row’s cellRenderQueue and cellColSpans in the rowsCache.
*
* @param {HTMLElement} divRow - The row container element to append the cell to.
* @param {number} row - The row index where the cell belongs.
* @param {number} cell - The column index of the cell.
* @param {number} colspan - The column span value for the cell.
* @param {number} rowspan - The row span value for the cell.
* @param {ColumnMetadata | null} columnMetadata - The metadata associated with the column, if available.
* @param {TData} item - The data item corresponding to the row.
*/
protected appendCellHtml(
divRow: HTMLElement,
row: number,
cell: number,
colspan: number,
rowspan: number,
columnMetadata: ColumnMetadata | null,
item: TData
) {
const m = this.columns[cell];
let cellCss = `slick-cell l${cell} r${Math.min(this.columns.length - 1, cell + colspan - 1)}` +
(m.cssClass ? ` ${m.cssClass}` : '') +
(rowspan > 1 ? ' rowspan' : '') +
(columnMetadata?.cssClass ? ` ${columnMetadata.cssClass}` : '');
if (this.hasFrozenColumns() && cell <= this._options.frozenColumn!) {
cellCss += ' frozen';
}
if (row === this.activeRow && cell === this.activeCell && this._options.showCellSelection) {
cellCss += ' active';
}
// Append additional CSS classes from cellCssClasses.
for (const key in this.cellCssClasses) {
if (
this.cellCssClasses[key] &&
this.cellCssClasses[key][row] &&
this.cellCssClasses[key][row][m.id]
) {
cellCss += ' ' + this.cellCssClasses[key][row][m.id];
}
}
let value: any = null;
let formatterResult: FormatterResultWithHtml | FormatterResultWithText | HTMLElement | DocumentFragment | string = '';
if (item) {
value = this.getDataItemValueForColumn(item, m);
formatterResult = this.getFormatter(row, m)(
row,
cell,
value,
m,
item,
this as unknown as SlickGridModel
);
if (formatterResult === null || formatterResult === undefined) {
formatterResult = '';
}
}
// Get additional CSS classes from the onBeforeAppendCell event and formatter result.
const evt = this.trigger(this.onBeforeAppendCell, { row, cell, value, dataContext: item });
const appendCellResult = evt.getReturnValue();
let addlCssClasses = typeof appendCellResult === 'string' ? appendCellResult : '';
if ((formatterResult as FormatterResultObject)?.addClasses) {
addlCssClasses += ' ' + (formatterResult as FormatterResultObject).addClasses;
}
const toolTipText = (formatterResult as FormatterResultObject)?.toolTip
? `${(formatterResult as FormatterResultObject).toolTip}`
: '';
const cellDiv = Utils.createDomElement('div', {
className: Utils.classNameToList(`${cellCss} ${addlCssClasses}`).join(' '),
role: 'gridcell',
tabIndex: -1
});
cellDiv.setAttribute('aria-describedby', this.uid + m.id);
if (toolTipText) {
cellDiv.setAttribute('title', toolTipText);
}
// Update cell rowspan height when spanning more than 1 row.
const cellHeight = this.getCellHeight(row, rowspan);
if (rowspan > 1 && cellHeight !== (this._options.rowHeight! - this.cellHeightDiff)) {
cellDiv.style.height = `${cellHeight}px`;
}
if (m.cellAttrs && typeof m.cellAttrs === 'object') {
for (const key in m.cellAttrs) {
if (m.cellAttrs.hasOwnProperty(key)) {
cellDiv.setAttribute(key, m.cellAttrs[key]);
}
}
}
// If there is a corresponding row (or this is the Add New row / data not yet loaded).
if (item) {
// Extract cell result from formatter. Ensure the value is one of string, HTMLElement or DocumentFragment.
const cellResult =
Object.prototype.toString.call(formatterResult) !== '[object Object]'
? formatterResult
: (formatterResult as FormatterResultWithHtml).html ||
(formatterResult as FormatterResultWithText).text;
this.applyHtmlCode(cellDiv, cellResult as string | HTMLElement | DocumentFragment);
}
divRow.appendChild(cellDiv);
this.rowsCache[row].cellRenderQueue.push(cell);
this.rowsCache[row].cellColSpans[cell] = colspan;
}
Failing tests:
should open the 1st Row Detail of the 2nd row and expect to find some detailsfailed
test body
(xhr)GET 200 /browser-sync/socket.io/?EIO=4&transport=polling&t=PJk_dn9&sid=0XmkW-Fl-KVaPvx9AABX
(xhr)POST 200 /browser-sync/socket.io/?EIO=4&transport=polling&t=PJk_dnB&sid=0XmkW-Fl-KVaPvx9AABX
1
get.slick-cell.detailView-toggle:nth(1)
2
click
(xhr)POST 200 /browser-sync/socket.io/?EIO=4&transport=polling&t=PJk_dno&sid=0XmkW-Fl-KVaPvx9AABX
3
wait50
4
get.slick-cell + .dynamic-cell-detail .detailViewContainer_10
AssertionError
Timed out retrying after 4000ms: Expected to find element: .slick-cell + .dynamic-cell-detail .detailViewContainer_1, but never found it.
[e2e/example16-row-detail.cy.ts:19:1](http://localhost:8080/__/#)
17 | .click()
18 | .wait(50);
> 19 | cy.get('.slick-cell + .dynamic-cell-detail .detailViewContainer_1')
| ^
20 | .find('h4')
21 | .should('contain', 'Task 1');
22 | cy.get('input[id="assignee_1"]')
View stack trace
Print to console
should open the 2nd Row Detail of the 4th row and expect to find some detailsfailed
test body
1
get.slick-row[style*="top: 250px;"] .slick-cell:nth(1)
2
click
3
wait50
4
get.slick-cell + .dynamic-cell-detail .detailViewContainer_30
AssertionError
Timed out retrying after 4000ms: Expected to find element: .slick-cell + .dynamic-cell-detail .detailViewContainer_3, but never found it.
[e2e/example16-row-detail.cy.ts:31:1](http://localhost:8080/__/#)
29 | .click()
30 | .wait(50);
> 31 | cy.get('.slick-cell + .dynamic-cell-detail .detailViewContainer_3')
| ^
32 | .find('h4')
33 | .should('contain', 'Task 3');
34 | cy.get('input[id="assignee_3"]')
View stack trace
Print to console
should be able to collapse all row detailsfailed
test body
1
get.dynamic-cell-detail0
2
assertexpected undefined to have a length of 2 but got 0
AssertionError
Timed out retrying after 4000ms: Expected to find element: .dynamic-cell-detail, but never found it.
[e2e/example16-row-detail.cy.ts:40:1](http://localhost:8080/__/#)
38 | });
39 | it('should be able to collapse all row details', function () {
> 40 | cy.get('.dynamic-cell-detail').should('have.length', 2);
| ^
41 | cy.get('[data-test="collapse-all-btn"]').click();
42 | cy.get('.dynamic-cell-detail').should('have.length', 0);
43 | });
View stack trace
Print to console
should open the Task 3 Row Detail and still expect same detailfailed
test body
1
get.slick-row[style*="top: 75px;"] .slick-cell:nth(1)
2
click
3
wait50
4
get.dynamic-cell-detail0
5
assertexpected undefined to have a length of 1 but got 0
AssertionError
Timed out retrying after 4000ms: Expected to find element: .dynamic-cell-detail, but never found it.
[e2e/example16-row-detail.cy.ts:48:1](http://localhost:8080/__/#)
46 | .click()
47 | .wait(50);
> 48 | cy.get('.dynamic-cell-detail').should('have.length', 1);
| ^
49 | cy.get('.slick-cell + .dynamic-cell-detail .innerDetailView_3')
50 | .find('h4')
51 | .should('contain', 'Task 3');
View stack trace
Print to console
should click on "click me" and expect an Alert to show the Help textfailed
test body
1
containsClick Me
0
AssertionError
Timed out retrying after 4000ms: Expected to find content: 'Click Me' but never did.
[e2e/example16-row-detail.cy.ts:59:1](http://localhost:8080/__/#)
57 | expect(str).to.contain('Assignee is');
58 | });
> 59 | cy.contains('Click Me')
| ^
60 | .click();
61 | });
62 | });
View stack trace
Nice to see you're making progress in improving the performance of the grid. I can't see the videos though, probably because I'm not logged in. The failing tests all seems to be for the Row Detail plugin, which is a bit special, it pushes all the next rows further down when the Row Detail is expanded (open), so this might need adjustment with the cache that you mentioned.
Hmm that's helpful. Sounds like it's a special case that needs targeted handling there.
Here's a gif snippet, that funnily enough accentuates the difference due to the lower frame rate.
Here's a gif snippet, that funnily enough accentuates the difference due to the lower frame rate.
that does seem a lot better on the UI side, wow nice job. I assume your improved code is on the left side... correct? Also curious, how large is your dataset? is that 10 million rows like you mentioned when you've opened the issue?