Too much recursion occurs when all-rows-visible is used with no explicit height

[abdullahtellioglu]

Description

When you have a grid with all-rows-visible attribute, you must set explicit height otherwise you might get maximum recursion or Maximum call stack size exceeded.

It also occurs with height: 100%

It is not thrown if you set the height to the grid.

Expected outcome

I would expect such a scenario not to throw an exception. Even though grid height is not set explicitly, it is not needed to render all components. Height can be received from computed styles.

Minimal reproducible example

import {css, html, LitElement} from "lit";
import { customElement, query, state } from 'lit/decorators.js';
import type { GridColumnBodyLitRenderer } from '@vaadin/grid/lit.js';
import { columnBodyRenderer } from '@vaadin/grid/lit.js';
import {copilotTree, CopilotTreeNode} from "Frontend/sample/tree";

export class OutlineSampleTest extends LitElement {

    @state()
    private collapsedItemUuids: Set<string> = new Set<string>();

    @query('vaadin-grid')
    private grid: any;

    @query('vaadin-context-menu')
    componentContextMenu!: any;

    @state()
    private contextMenuItems = [{ text: 'View' }, { text: 'Edit' }, { text: 'Delete' }];

    constructor(){
        super();
        copilotTree.listener(() => this.componentTreeUpdated());
    }
    render() {
        return html`
      <vaadin-context-menu
        .items="${this.contextMenuItems}">
        <vaadin-grid
          rows-draggable
          all-rows-visible
          .itemIdPath="${'uuid'}"
          .expandedItems="${copilotTree.allNodesFlat.filter((node) => !this.collapsedItemUuids.has(node.uuid))}"
          .dataProvider=${this.dataProvider}
          drop-mode="on-top-or-between"
          @vaadin-contextmenu="${this.onContextMenu}"
          theme="no-border no-row-borders">
          <vaadin-grid-column auto-width ${columnBodyRenderer(this.renderer, [])}></vaadin-grid-column>
        </vaadin-grid>
      </vaadin-context-menu>
    `;
    }
    private renderer: GridColumnBodyLitRenderer<CopilotTreeNode> = (node, model) => {
        return html`
      <vaadin-grid-tree-toggle
        uuid="${node.uuid}"
        .leaf="${node.children.length === 0}"
        .level="${model.level}"
        .selected="${!!model.selected}"
        .expanded="${!!model.expanded}"
        @click=${(e: MouseEvent) => {
            const expanded = (e.currentTarget as any).expanded;
            if (expanded) {
                // Expanded
                this.collapsedItemUuids.delete(node.uuid);
            } else {
                this.collapsedItemUuids.add(node.uuid);
            }
            this.requestUpdate();
        }}>
        <label title="${node.title}">${node.uuid}</label>\`
      </vaadin-grid-tree-toggle>
    `;
    };

    async onContextMenu(e: MouseEvent) {
        e.preventDefault();
        e.stopPropagation();
    }
    private componentTreeUpdated() {
        if (this.grid) {
            this.grid.clearCache();
            this.requestUpdate();
        }
    }

    index = 0;
    private dataProvider = async (
        params: any, // GridDataProviderParams<ComponentTreeItem>,
        callback: any, // GridDataProviderCallback<ComponentTreeItem>
    ) => {
        if (!copilotTree.root) {
            callback([], 0);
        } else if (!params.parentItem) {
            callback([copilotTree.root], 1);
        } else {
            const parent = params.parentItem as CopilotTreeNode;
            const children = copilotTree.getChildren(parent.uuid);
            console.log(parent.uuid, this.index ++);
            callback(children, children.length);
        }
    };
}
customElements.define('outline-sample', OutlineSampleTest);

Use this component to populate data.

export type CopilotTreeNode = {
    title: string,
    uuid: string,
    parent?: CopilotTreeNode,
    children: CopilotTreeNode[],
}

export class CopilotTree {
    allNodesFlat: CopilotTreeNode[] = [];

    root?: CopilotTreeNode;

    createIndex = 0;

    listenerMethod: () => void;
    create(){
        this.allNodesFlat = [];
        this.createIndex = 0;
        this.root = this.createNode(this.createIndex++);
        this.addChild(this.root);
        if(this.listenerMethod){
            this.listenerMethod();
        }
    }

    addChild(parent: CopilotTreeNode) {
        if(this.createIndex > 1000){
            return;
        }
        const rndInt = Math.floor(Math.random() * 6) + 1
        const addedChildren = [];
        for(let i =0;i<rndInt;i++){
            let child = this.createNode(this.createIndex++, parent);
            child.parent = parent;
            child.parent.children.push(child);
            addedChildren.push(child);
        }
        addedChildren.forEach(child => this.addChild(child));
    }
    createNode(index: number, parent?: CopilotTreeNode): CopilotTreeNode {
        const node = {
            parent,
            get uuid(){

                return `${index}`;
            },
            get title(){
                return `${index}`;
            },
            children: [],
        } as CopilotTreeNode;
        if(!parent){
            this.root = node;
        }
        this.allNodesFlat.push(node);
        return node;
    }
    getChildren(uuid: string): CopilotTreeNode[] {
        return this.allNodesFlat.find(k => k.uuid === uuid)!.children;
    }
    listener(method: () => void){
        this.listenerMethod = method;
    }
}

export const copilotTree = new CopilotTree();
copilotTree.create();

Steps to reproduce

Copy the following code above to an example project and open the browser and you will see that lots of exception is logged in the console.

Environment

Hilla: 24.5.0.alpha4 Flow: 24.5.0.alpha8 Vaadin: 24.5.0.alpha4 Copilot: 24.5-SNAPSHOT Copilot IDE Plugin: 1.0.9-intellij Java: JetBrains s.r.o. 21.0.3 Java Hotswap: false Frontend Hotswap: Enabled, using Vite OS: aarch64 Mac OS X 14.5 Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/127.0.0.0 Safari/537.36

Browsers

No response

[yuriy-fix]

We would need to verify if there is a bug in the example provided or in the component.

[abdullahtellioglu]

FYI, there is a conversation in slack about this topic from January. Implementing what Tomi suggested(adding height:100% along with removing all-rows-visible) does not work for grids that have many rows ( 'many' depends on the browser. Chrome throws the exception somewhere between 60-70 elements, and Firefox is more than that)

[tomivirkki]

I tried the provided example locally. The purpose of all-rows-visible is that it makes the grid's height unlimited / not fixed, so it grows large enough to cover all the items available. In this case, the data provider returns an infinite amount of items so an exception is quite expected if you use all-rows-visible together with it.

The API documentation of allRowsVisible also mentions that it shouldn't be used with a large number of items: "Effectively, this disables the grid's virtual scrolling so that all the rows are rendered in the DOM at once. If the grid has a large number of items, using the feature is discouraged to avoid performance issues."

It also occurs with height: 100%

There's no exception using height: 100% on the grid as long as its parent has a defined height. Try using display: block; height: 800px; on the outline-sample element for example. Currently, outline-sample has no display defined at all.

If you still don't want to define a height for the container and want to use all-rows-visible, you should at least specify a max-height for the grid to have it stop growing once it reaches a certain height.

[abdullahtellioglu]

In the example below, grid has 800 px height and max-height but the exception still happens.

https://github.com/user-attachments/assets/8c0f7521-5c81-4ba8-a98c-42af64d914e5

The grid has less than 100 rows in total.

[tomivirkki]

How do I reproduce this issue? It doesn't happen with the original example (with 800px height applied)

[abdullahtellioglu]

You can reproduce it with the following styles

                vaadin-grid {
                    --vaadin-grid-cell-padding: 0 0;
                    max-height: 1000px;
                }
                vaadin-grid::part(cell) {
                    min-height: auto;
                }

[tomivirkki]

The error message only shows on Chrome. FF and Safari seem to have higher limits on how much recursion they tolerate.

[vursen]

The workaround is to make your data provider asynchronous, for example by wrapping callback(items, size) in a setTimeout. This will prevent the recursion that occurs because the grid requests children for all uncached expanded rows each time it receives a page response:

https://github.com/vaadin/web-components/blob/813890c689aba55951ac099e709d7c5d60f7c8e5/packages/grid/src/vaadin-grid-data-provider-mixin.js#L344-L346

P.S. Here is a simplified reproduction:

<script type="module">
  import '@vaadin/grid/all-imports';

  const grid = document.querySelector('vaadin-grid');

  grid.dataProvider = ({ parentItem, page, pageSize }, cb) => {
    // Let's have 100 root-level items and 5 items on every child level
    const levelSize = parentItem ? 5 : 50;

    const pageItems = [...Array(Math.min(levelSize, pageSize))].map((_, i) => {
      const indexInLevel = page * pageSize + i;

      return {
        name: `${parentItem ? parentItem.name + '-' : ''}${indexInLevel}`,
        children: true,
      };
    });

    cb(pageItems, levelSize);
  };

  grid.expandedItems = [...Array(50)].map((_, i) => {
    return { name: `${i}` };
  });
</script>

<vaadin-grid item-id-path="name" all-rows-visible>
  <vaadin-grid-tree-column frozen path="name" width="200px" flex-shrink="0"></vaadin-grid-tree-column>
</vaadin-grid>