checkbox has wrong value inside grid column

[KardonskyRoman]

Describe the bug

There is a checbox inside a grid column. It has value according to item.status. The table shows only items which item.status = true. If I click to the first item, its value item.status becomes false and the item is hidden (as expected), but second item is rendered with unchecked component, however its value is true.

@state()
  private items: Array<{ name: string; status: boolean }> = [
    {
      name: "John",
      status: true,
    },
    {
      name: "Mike",
      status: true,
    },
  ];

 @query("#grid")
  private grid!: Grid;

...
<vaadin-grid-column
          header="Status"
          ${columnBodyRenderer<Record<string, string | boolean>>(
            (item) => html`
              <vaadin-checkbox
                .checked=${Boolean(item.status)}
                @click=${async (e: MouseEvent) => {
                  item.status = (e.target as HTMLInputElement).checked;
                  this.grid.clearCache();
                }}
              >
              </vaadin-checkbox>
            `,
            []
          ) as DirectiveResult}
        ></vaadin-grid-column>
...
async dataProvider(
    params: GridDataProviderParams<any>,
    callback: GridDataProviderCallback<any>
  ) {
    const result = this.items?.filter((item) => {
      return item.status == true;
    });

    callback(result ?? [], result?.length);
  }

Expected-behavior

checkbox should show correct value

Reproduction

The reproducable example in https://github.com/KardonskyRoman/hilla_test

System Info

hilla 2.5.6

[sissbruecker]

Should be a web component issue, moving it over there.

[KardonskyRoman]

I am not sure this is checkBox component problem, looks like columnBodyRenderer provides old value

[sissbruecker]

columnBodyRenderer is part of the @vaadin/grid web component package, so we are in the correct repo now.

[sissbruecker]

Some debugging:

• The checkbox is initially rendered with .checked={true}
• Clicking it toggles its checked state
• Then the click listener causes a row to be removed, the toggled checkbox is now reused for a different row
• The same checkbox is now rendered again with .checked={true}
• Lit doesn't seem to do anything in that case because the checkbox was not rendered with .checked={false} in between

A more basic reproduction is:

• Render a checkbox using Lit:

  render(html`<input type="checkbox" .checked=${true}>`, container)

• Toggle the checkbox to off
• Render the checkbox again:

  render(html`<input type="checkbox" .checked=${true}>`, container)

• Checkbox is still off

Probably how Lit should behave, but problematic for grid where Lit templates can end up rendering different items on each content update.

At least in this case the issue can be fixed by forcing the grid to re-render its rows before removing the row:

item.status = (e.target as HTMLInputElement).checked;
this.grid.requestContentUpdate();
this.grid.clearCache();

[KardonskyRoman]

Hmm, your solution works in provided example, but doesn't work in real project. The difference is that items are loaded from endpoint.

[KardonskyRoman]

I updated the reproduction example in https://github.com/KardonskyRoman/hilla_test with fetching data from an endpoint. Now your solution dosn't work.

[sissbruecker]

You need to do the same thing as in the example when the checkbox is toggled:

• Change the item's status on the client side as well, in addition to doing it on the server
• Call requestContentUpdate immediately afterwards

Edit: Actually you can keep requestContentUpdate in your update button click listener, but you need to update the state on the client, otherwise the render will not update the state of the Lit element.

[KardonskyRoman]

It is clear, thanks. You can close it, if this is not a bug.

[sissbruecker]

I'd say it's a bug, ideally grid should somehow deal with the fact that elements rendered with Lit can be reused in different rows.

[DiegoCardoso]

As @sissbruecker has mentioned, this is how Lit render function works.

In summary, once Lit renders the template for the first time, it keeps track of all the values assigned to the properties/attributes. When the user interacts with the input and changes its value, that change is not reflected in the internal state Lit of the committed value it has.

In Grid, it reuses the cell whenever there's a change in the items (for instance, when a new page is fetched while scrolling). So whenever the renderer is called for a reused cell, Lit will first check if the template provided is the same as the one already assigned to that element and, if so, will proceed with the checks for all the values passed to it.

In your example, even though the <vaadin-checkbox> has a different checked value (since the user has unchecked it), Lit has no way of knowing it, so when it receives .checked=${true} again, it checks its internal state and finds that the value hasn't changed from the last time render was called and, therefore, there's no need to update the value.

Lit, however, provides a directive called live, which checks the element's current value against the one provided by the template and, in case they differ, it updates the value.

So, you could update your <vaadin-grid-column> to:

// importing the directive
import {live} from 'lit/directives/live.js'

// ...
<vaadin-grid-column
          header="Status"
          ${columnBodyRenderer<Record<string, string | boolean>>(
            (item) => html`
              <vaadin-checkbox
                .checked=${live(Boolean(item.status))}
                @click=${async (e: MouseEvent) => {
                  item.status = (e.target as HTMLInputElement).checked;
                  this.grid.clearCache();
                }}
              >
              </vaadin-checkbox>
            `,
            []
          ) as DirectiveResult}
        ></vaadin-grid-column>

@KardonskyRoman can you try if this changes solves your issue?

[KardonskyRoman]

Hello, thank you for the answer. I already implemented the workaround which @sissbruecker suggested.

[vursen]

It's technically possible to prevent such kind of binding issues on the web component side by clearing the rendered content not only when the renderer function changes but also when it's called with a different item (possible to find out by comparing the results of getItemId):

https://github.com/vaadin/web-components/blob/feb975686471b588dbbb11418d9bf1bf33f22022/packages/grid/src/vaadin-grid-mixin.js#L931-L936

https://github.com/vaadin/web-components/blob/feb975686471b588dbbb11418d9bf1bf33f22022/packages/grid/src/vaadin-grid-column-mixin.js#L652-L658

However, there is a downside that this approach may negatively affect rendering performance, as it prevents Lit from reusing DOM elements. From this perspective, using live directive would offer better performance, but its choice may be not always obvious indeed.

Example that I used to confirm that live directive solves the issue as well:

<script type="module">
  import { render, html } from 'lit';
  import { live } from 'lit/directives/live.js';
  import '@vaadin/grid/all-imports';

  const grid = document.querySelector('vaadin-grid');
  grid.items = ['Item 1', 'Item 2'];

  const column = document.querySelector('vaadin-grid-column');
  column.renderer = (root, column, model) => {
    render(
      html`
        <vaadin-checkbox
          .checked=${live(true)}
          @change=${(e) => {
            grid.items = ['Item 1'];
          }}
        ></vaadin-checkbox>
      `,
      root
    );
  }
</script>

<vaadin-grid>
  <vaadin-grid-column></vaadin-grid-column>
</vaadin-grid>