vaadin / flow-components

Java counterpart of Vaadin Web Components
99 stars 66 forks source link

Vaadin Grid with Lit renderers: "Cannot read properties of undefined" when switching out columns and data #6426

Closed tigerasks closed 2 months ago

tigerasks commented 2 months ago

Description

When switching out Grid columns and rows, Vaadin calls the grid columns' lit renderers with undefined properties, resulting in a "Cannot read properties of undefined" error.

I do not think that developers are easily aware of this or should have to account for the possibility of this happening.

Expected outcome

In the absence of computable item properties, Lit renderer should not be invoked.

Minimal reproducible example

package com.tigerasks.demo

import com.tigerasks.demo.LitIssueMVP.RenderProperty.VALUE_LIST
import com.tigerasks.layout.MainLayout
import com.vaadin.flow.component.Composite
import com.vaadin.flow.component.button.Button
import com.vaadin.flow.component.dependency.JsModule
import com.vaadin.flow.component.grid.Grid
import com.vaadin.flow.component.orderedlayout.VerticalLayout
import com.vaadin.flow.data.provider.DataProvider
import com.vaadin.flow.data.provider.Query
import com.vaadin.flow.data.renderer.LitRenderer
import com.vaadin.flow.router.Route

private typealias Row = Map<String, List<String>>

@Route("ui/demo/lit-renderer-issue-mvp", layout = MainLayout::class)
@JsModule("./lit/column-renderers/issue-renderers.js")
class LitIssueMVP : Composite<VerticalLayout>() {
    private object RenderProperty {
        const val VALUE_LIST = "values"
    }

    companion object {
        private const val TEXT_TEMPLATE = "${'$'}{litIssue.textRenderer(item.$VALUE_LIST)}"
    }

    private val toggleDataButton = Button("Toggle data").apply {
        addClickListener {
            toggleData()
        }
    }

    private val data: DataProvider<Row, Void> = DataProvider.fromCallbacks(
        { query ->
            println("FETCH: ${query.offset} ${query.limit}") //access offset, limit to avoid contract violation
            fetch(query)
        },
        { query ->
            println("COUNT: ${query.offset} ${query.limit}") //access offset, limit to avoid contract violation
            count(query)
        },
    )

    val grid = Grid<Row>().apply {
        setItems(data)
    }

    private fun Grid<Row>.addAllColumns(columns: List<String>) {
        columns.forEach { column ->
            addColumn(
                LitRenderer
                    .of<Row>(TEXT_TEMPLATE)
                    .withProperty(VALUE_LIST) { row -> row[column] }
            ).apply {
                setHeader(column)
                setKey(column)
            }
        }
    }

    override fun initContent() = VerticalLayout().apply {
        style["padding"] = "0"
        add(
            toggleDataButton,
            grid,
        )
    }.also {
        refreshData()
    }

    object MockData {
        private val columnsA = listOf("id", "A1", "A2", "A3")
        private val rowsA = listOf(
            mapOf(
                "id" to listOf("Hello"),
                "A1" to listOf("A1R1", "A1R2"),
                "A2" to listOf("A2R1", "A2R2"),
                "A3" to listOf("A3R1", "A3R2"),
            ),
            mapOf(
                "id" to listOf("World"),
                "A1" to listOf("A1R3", "A1R4"),
                "A2" to listOf("A2R3", "A2R4"),
                "A3" to listOf("A3R3", "A3R4"),
            ),
        )

        private val columnsB = listOf("id", "B1", "B2")
        private val rowsB = listOf(
            mapOf(
                "id" to listOf("Alpha"),
                "B1" to listOf("B1R1", "B1R2"),
                "B2" to listOf("B2R1", "B2R2"),
            ),
            mapOf(
                "id" to listOf("Beta"),
                "B1" to listOf("B1R3", "B1R4"),
                "B2" to listOf("B2R3", "B2R4"),
            ),
            mapOf(
                "id" to listOf("Gamma"),
                "B1" to listOf("B1R3", "B1R4"),
                "B2" to listOf("B2R3", "B2R4"),
            ),
        )

        private var toggleCount = 0

        fun toggle() {
            toggleCount++
        }

        val columns
            get() = when (toggleCount % 2) {
                0 -> columnsA
                else -> columnsB
            }

        val rows
            get() = when (toggleCount % 2) {
                0 -> rowsA
                else -> rowsB
            }
    }

    private fun fetch(query: Query<Row, *>) = MockData
        .rows
        .stream()

    private fun count(query: Query<Row, *>) = MockData
        .rows
        .count()

    private fun toggleData() {
        MockData.toggle()
        refreshData()
    }

    private fun refreshData() {
        grid.removeAllColumns()
        grid.addAllColumns(MockData.columns)
    }
}

where issue-renderes.js is

import {html} from "lit";
import {until} from "lit/directives/until.js";

window.litIssue = window.litIssue || {};

litIssue.textRenderer = function (rowValues) {
    return litIssue.multipleValuesRenderer(
        rowValues,
        async (row, _) =>
            html`<span>${row}</span>`,
    );
}

litIssue.multipleValuesRenderer = function (rowValues, valueRenderer) {
    switch (rowValues.length) {
        case 0:
            return html`EMPTY`;
        case 1:
            return html`
                ${
                    until(
                        valueRenderer(rowValues[0], 0),
                        html`<span>...</span>`
                    )
                }
            `;
        default:
            let individualValues = Promise.all(
                rowValues.map(async (row, i) =>
                    html`
                        <li>${await valueRenderer(row, i)}</li>`
                )
            );

            return html`
                ${
                    until(
                        individualValues.then(items =>
                            html`
                                <ul>${items}</ul>
                            `
                        ),
                        html`<span>...</span>`
                    )
                }
            `
    }
}

Steps to reproduce

REPRODUCE

IS image

SHOULD Not raise an error and display the new columns and data, instead.

WORKAROUND Change

private const val TEXT_TEMPLATE = "${'$'}{litIssue.textRenderer(item.$VALUE_LIST)}"

to

private const val TEXT_TEMPLATE = "${'$'}{litIssue.textRenderer(item.$VALUE_LIST || [])}"

Environment

Vaadin version(s): 24.2.10 OS: Ubuntu 22.04.4 LTS

Browsers

Chrome

sissbruecker commented 2 months ago

This seems to be fixed in Vaadin 24.4. Since 24.2 is not maintained anymore, I'd suggest to upgrade.

I've been using this reproduction:

@Route("repro")
public class Repro extends Div {
    private final Grid<Integer> grid;

    public Repro() {
        grid = new Grid<>();
        grid.setItems(1, 2, 3, 4, 5);
        grid.addColumn(createLitRenderer()).setHeader("Number of array items");

        var refreshButton = new NativeButton("Refresh", e -> refresh());
        add(grid, refreshButton);
    }

    void refresh() {
        grid.setItems(1, 2, 3, 4, 5, 7, 8);
        grid.removeAllColumns();
        grid.addColumn(createLitRenderer()).setHeader("Number of array items");
    }

    Renderer<Integer> createLitRenderer() {
        return LitRenderer.<Integer>of("<span>${item.values.length}</span>")
                .withProperty("values", v -> IntStream.range(0, v).toArray());
    }
}