Props updated on table-column does not updating table component

[maorbote]

When binding data on table-column, table would not be updated as data is updating.

Here is a simple demo.
https://jsfiddle.net/p0fe9s06/5/

I have done some research and found Evan You(@yyx990803) has mention in vuejs/vue#4332

It seems you are trying to make a slot container communicate with a slot child - in most cases this means the two components are coupled by-design, so you can do something like this.$parent.$emit(...) from the child, and listen to that event in the parent with this.$on(...).

So I try to add watch in the TableColumn

watch: {
    show() {
        this.$parent.$emit('columnUpdate');
    },
    label() {
        this.$parent.$emit('columnUpdate');
    },
    headerClass() {
        this.$parent.$emit('columnUpdate');
    },
    cellClass() {
        this.$parent.$emit('columnUpdate');
    },
    hidden() {
        this.$parent.$emit('columnUpdate');
    },
    filterable() {
        this.$parent.$emit('columnUpdate');
    },
...

and this in the TableComponent

async mounted() {
    this.columns = this.$slots.default
        .filter(column => column.componentInstance)
        .map(column => new Column(column.componentInstance));

    await this.mapDataToRows();

+   this.$on('columnUpdate', () => {
+       this.columns = this.$slots.default
+           .filter(column => column.componentInstance)
+           .map(column => new Column(column.componentInstance));
+
+       this.mapDataToRows();
+   });
},

Although it works fine, but it's not seems like an elegant approch.

Does anybody have a clue about this?

[sebastiandedeyne]

I think this is gonna be a necessary evil for the way this package is set up. I'll look into adding this later this week.

The first snippet won't need to be that verbose though, you can pass an array to the $watch method.

created() {
    this.$watch(['show', 'label', ...], () => {
        this.$parent.$emit('columnUpdate');
    });
},

[maorbote]

Wow, thanks for the snippet. I don't know that this.$watch can pass in array.

I was thinking the columns can be somehow updated one by one instead of all columns, but since any props might be changed, it's difficlut to tracks which column has changed.

The column might even been removed from or inserted into the table. so what really necessary is watching the changes of the $slots, but it seems there is no other way to do this :(

I'm looking forward to the future update. Cheers, bro!

[sebastiandedeyne]

First of all, sorry I lied 😅 I really thought I passed an array to $watch before, but apparently it's not a feature after all 😞 (see vuejs/vue#844)

Imo it's easier to just use a single change event. I see swapping columns as an edge case, so I don't think we should be optimizing too hard at this stage.

I just released 1.4.3 which contains your proposed fix. I structurd it a bit different than you proposed, everything's defined on the parent component, and I'm looping over the properties so we never miss anything.

Here's the commit: 3c9aee8

[maorbote]

My apologies for late comment, I found 2 issues on this commit 3c9aee8

columnComponents.forEach(column => {
    Object.keys(this.$options.props).forEach(
         prop => this.$watch(prop, () => this.mapColumns())
    );
});

1. this in this.$options.props, this.$watch, this.mapColumns() is actually points to TableComponent when using arrow function, so it is in fact watching TableComponent's props instead of TableColumn.

2. It will bind watch function multiple times since mapColumns() is calling itself(recursive), the browser will hangs if the prop changes frequently(I tried).

The mysterious thing is although it is watching incorrect props but it still works!
I performed some experiments and found out it only needs to watch data.

    async mounted() {
        this.mapColumns();

        await this.mapDataToRows();
+       this.$watch('data', () => this.mapColumns());
    },

    mapColumns() {
        const columnComponents = this.$slots.default.filter(
            column => column.componentInstance
        );

        this.columns = columnComponents.map(
            column => new Column(column.componentInstance)
        );

-       columnComponents.forEach(column => {
-           Object.keys(this.$options.props).forEach(
-                prop => this.$watch(prop, () => this.mapColumns())
-           );
-       });
    },

Well, I don't understand but it just works.

One more thing.
It needs to warp with setTimeout if adding mapColumns() into watch, I don't get this as well.

watch: {
    data() {
+       // this.mapColumns(); // don't works
+       setTimeout(() => this.mapColumns(), 0);
+
            if (this.usesLocalData) {
            this.mapDataToRows();
        }
    },
},

(Confused Jackie Chan face)

[sebastiandedeyne]

You're right about watching those props, and I also have no idea why they work now 😄

I'll look into this issue again later this week :)

[maorbote]

Hey, is there any progress of this issue?

I had submit a PR #52 for this, it will be very appreciated if you could spend some time for it.

Great thanks.

[sebastiandedeyne]

Hi! Was on holiday so a bit later on this one :)

Looks good to me, merged and tagged a fresh 1.6.1 release!