add 'prop' property to columnFilters

[sly7-7]

Hi, I wanted to use the async loading + filtering on columns. I think it would be easier to build the filter params if we had the property name passed to the column filters. What do you think ?

[cah-brian-gantzler]

Previously I guess you would have had to code the order of the columns in to your filter logic to to even use this. Moving columns around would have meant needing to change the filtering as well. Doing it this way removes the coupling of that logic.

I like it.

[miguelcobain]

It's not related to this PR, but I'm worried about the failing tests on ember beta and canary.

[sly7-7]

Yes, I saw that too, but I don't know what to do. I've already seen that kind of message in my app, and was wondering how to fix :(

[sly7-7]

Maybe we will have more insight on what's going on, when this ember-learn/guides-source#1528 will be explained in details
Maybe @rwjblue and/or @pzuraq could give us some advices ?

[cah-brian-gantzler]

The above mentioned guide basically tells you what not to do, but doesn't say how to do it the right way. The above is also an example of a constructor, and I think I know how I would avoid that.

This problem is the opposite, this is actually happening during the destruction, specifically when the columns are unregistering themselves, this is something that has to happen. when columns are hidden, but when the table is being torn down, the columns dont really need to unregister, because the table it going away, the problem is the columns are being torn down before the table, so we dont know we can skip the unregister.

I dont have a suggestion on this is suppose to happen. Perhaps we have to look into the usage of https://github.com/ember-polyfills/ember-destroyable-polyfill

You can see below the columns is referred to and updated. Based on the above guide, this works fine during normal execution where columns are added and removed, but during complete table teardown we must in the the tracking frame @rwjblue is talking about

 unregisterColumn(column) {
      let columns = this.get('columns');

      if (columns.includes(column)) {
        this.set('columns', columns.filter(c => c !== column));
      }
    }

[miguelcobain]

@sly7-7 this is still an interesting feature. Would you mind updating this PR? Now we're using github actions and tests seem to be working on latest ember versions. 👍

[sly7-7]

@miguelcobain Of course I will try to update :) I'm not familiar at all with Github actions, if I have any problem, I'll ping you.

EDIT: Should be good to merge :)

[miguelcobain]

Thanks!