Thead::Row::Cell is not rendered in v2

[jelhan]

There is a timing error, which breaks usage of <Thead::Row::Cell>. The cell is not rendered.

This is a simplified use case, which I'm having:

<YetiTable
    @data={{array (hash firstName="Max" lastName="Mustermann") (hash firstName="John" lastName="Doe")}}
    as |table|
>
    <table.thead as |thead|>
        <thead.row as |row|>
            <row.column @prop="firstName">
                first name
            </row.column>
            <row.column @prop="lastName">
                last name
            </row.column>
        </thead.row>
        <thead.row as |row|>
            <row.cell>
                <input placeholder="first name">
            </row.cell>
            <row.cell>
                <input placeholder="last name">
            </row.cell>
        </thead.row>
    </table.thead>
    <table.body />
</YetiTable>

The second <thead.row>, which uses <row.cell> is not rendered.

I debugged it and noticed that this.args.columns is an empty array when the cell is registered here: https://github.com/miguelcobain/ember-yeti-table/blob/216485a33c8c0aff15b75f65f3dec494facd718c/addon/components/yeti-table/thead/row.gjs#L42-L58

The column is registered here: https://github.com/miguelcobain/ember-yeti-table/blob/216485a33c8c0aff15b75f65f3dec494facd718c/addon/components/yeti-table.gjs#L703-L713 That function is executed before registerCell. But the work is scheduled for afterRender, which is executed after registerCell.

Please find a reproduction here: https://github.com/jelhan/reproduction-ember-yeti-table-cell-registration-issue

[jelhan]

This is a regression in ember-yeti-table v2. It works as expected if downgrading to v1.7.2.

[cah-brian-gantzler]

V2 went from components and 2 way binding to glimmer components and DDAU. In many ways a lot of the control and flow was rewritten.

Im wondering why registerColumn is scheduled afterRender (or scheduled at all). Would have to look at that.

Currently there is a PR out https://github.com/miguelcobain/ember-yeti-table/pull/462 that we should get merged first. Can take a look at this after that. Thanks for the report.

[jelhan]

If we can remove the scheduling after render that would resolve the issue. Likely the best option to investigate first.

Also noticed that there were many changes between v1 and v2. Therefore I have up tracking it down to a specific commit.

[jelhan]

I tested if we can remove scheduling column registration on afterRender. But this leads to modification after consumption errors. Doesn't seem to be an easy fix. :cry:

I pushed the test, which I have written for the experiment in #465. May help for others testing out potential fixes as well.

[cah-brian-gantzler]

Thanks for confirming the reason the schedule is there. That particular error can be annoying to fix. I have that issue right now. Updating from ember 5.5 to 5.6 causes that issue in 3 tests for my application that I have to hunt down.

[jelhan]

Yeah. That indicates an architectural issue, which was worked around by scheduling outside of rendering loop.

If we are lucky, the only consumption is directly before modification: https://github.com/miguelcobain/ember-yeti-table/blob/216485a33c8c0aff15b75f65f3dec494facd718c/addon/components/yeti-table.gjs#L709-L711

Maybe we can convert columns to a TrackedSet. In that case we wouldn't need to check if it's already in the list.

As a next step we could check if columns actually need to be tracked.

[jelhan]

I tried using a TrackedSet but also run into the update after usage error.

Main implementation looked like this:

   registerColumn(column) {
-    schedule('afterRender', this, function () {
-      if (typeof this.args.isColumnVisible === 'function') {
-        column.visible = this.args.isColumnVisible(column);
-      }
+    if (typeof this.args.isColumnVisible === 'function') {
+      column.visible = this.args.isColumnVisible(column);
+    }

-      if (!this.columns.includes(column)) {
-        this.columns = [...this.columns, column];
-      }
-    });
+    this.columns.add(column);
   }

Update after usage error was thrown by this.columns.add(column).

Main learning from this attempt is that this.columns.includes(column) is not the only usage of columns in the rendering cycle.