Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

DataTables 1.9.1 breaks the bindings #9

Open
DataTables opened this Issue May 05, 2012 · 26 comments

9 participants

Allan Jardine Lucas Martin Anders Malmgren Tanguy Krotoff Miroslav Popovic memelet Alex Frenkel Tommy Holm Jakobsen Anton Romanov
Allan Jardine

Hi,

It would appear that a change I've made in DataTables 1.9.1+ has had the rather undesired effect of breaking this extension :-(. This thread in the DataTables forum discusses the issue: http://datatables.net/forums/discussion/9715 .

Basically the change that I made was to have DataTables call its 'set' function for data when the data is added to the table. This means that when you add data, DataTables is trying to set the value to itself (typically speaking) - sounds odd, but its really useful for allowing mDataProp to work as a function, rather than just a string. So the value in this case is whatever is being returned by the Knockout function, and then the data gets set to that - overwriting the Knockout function call in the data source!

In particular this is the debug trace from the test in the forum thread shows what is happening - http://debug.datatables.net/ahiwel . Click on the "Tables" tab and then show the "Initialisation options". Notice that in "aaData" there are functions to bind to Knockout. My change to DataTables means that those functions get overwritten with the static data...

I'm very much open to changing how this works in DataTables - I'm sorry that this has been an issue - but what I would suggest perhaps is a slightly different way of doing things if I may, which would also completely integrate the extension with the DataTables API (fnAddData, fnUpdate might cause issues at the moment for example).

So what I would suggest is to use mDataProp, not as a string as it currently is, but as a function - this blog post details how mDataProp can be used as a function: http://datatables.net/blog/Orthogonal_data .

So instead of:

"mDataProp": "user"

the code might be something like:

"mDataProp": function (data, type, val) {
    if ( type === "set" ) {
        data.user( val ); // ... call knockout to set the value for "user" to val
        return;
    }
    return data.user(); // ... call to knockout to get the value of "user"; ?
}

I'm afraid I haven't yet dug into knockout so I don't know what API calls should be being made there - I've just made something up a bit... - but that's the general idea.

Does that sound feasible? Sorry for creating extra work! (although mDataProp's flexibility when used in this way should make it worth it :-) ).

Allan Jardine

One thing that I've just thought of, related to this - at the moment DataTables makes a copy of the data passed to it - would it be helpful at all if this didn't happen? That way you can bind to a straight data source and, if it is manipulated by DataTables or Knockout, or anything else, it doesn't matter - all would see the same change in the data (although DataTables doesn't have an "observable" paradigm - so possibly this isn't all that useful in this case...

Lucas Martin
Owner
ducka commented May 10, 2012

Heya Allan, thanks for reporting this issue mate. I'll see if I can familiarise myself with this problem over the weekend.

Regarding DataTables copying it's data source... I noticed this behaviour a few months back in the demo project I created for this binding. Sample 2 of the demo project shows a table where each row has it's own "Remove" button that is bound by click to a "removeItem" function in the view model. When one of the remove buttons is clicked, the current data context for that table row (which is an item in the array data source for the table.) is passed to the removeItem function. Ideally to remove this item from the array, I would simply remove the item by reference; however considering dataTables copies the data source internally, I am unable to do this because the instance of the item passed to the removeItem function does not exist in the original data source array. To work around this issue, I've had to remove the item from the array by key (as shown below).

this.removeItem = function (item) {
    // Issue: DataTables is copying the dataSource of the data table internally, so an attempt to remove by instance here wont work.
    // Unfortunately we have to match by key here currently...
    //            this.data.remove(item);
    this.data.remove(function (item2) { return item.Id == item2.Id; });
}

I haven't lifted the lid on this binding and the DataTables library for a while, so I'm a little unfamilar with the situation at the moment. Having said that, I'm going to take a stab at the hypothetical you posted.

I have thought on the possibility of modifying DataTables to use the original datasource instead of copying it, but ultimately I let the idea go from lack of time to spend on the problem, and from the assumption that you probably copied the datasource for a good reason in the first place :) From what I recall, I couldn't see any reason why this solution couldn't work, but I also wasn't sure I wasn't about to break DataTables in a way that would make me cry. Having DataTables use the original view model as the datasource instead of copying it would make things easier for this binding, so if you're entertaining the idea of modifying DataTables to use the original datasource as opposed to copying it, I'd be happy to work with you on this to get it working better with Knockout.

Let me know what you think!

Anders Malmgren

Any status on this? I would really like to use a newer version of Datatables if possible

Allan Jardine

Hi - sorry for the delay in getting back to you ducka - thanks your thoughts on this.

The reason that DataTables is doing the clone is "bidirectional" - if it wasn't a cloned data array and you were externally to change the array - DataTables wouldn't know about that, and thus absolutely anything could go wrong - the data isn't what it is expecting. Equally, if DataTables were to modify the data source without you knowing about it, then your own processes that use the data would also likely be a bit upset.

So the key missing element is an 'observable' variable type in Javascript (perhaps like bindable in AS3?). I know that this is one of the really attractive features of Knockout - change your parameter and the views get updated etc - however, DataTables doesn't know anything about Knockout internally, and I'd be very reluctant to make Knockout a dependency for DataTables (not because I don't like it - just because I prefer to keep dependencies to a minimum!).

Given the overlap between Knockout and DataTables, and with no observable variables between them, I think (please anyone jump in with ideas!) cloning, and syncing the API calls between them is the only thing that can be done (delete from Knockout and delete from DataTables, or have one as the "master" which will inform the other of an action).

Having said that - I can see that not copying the data source could be very useful. An API call to tell DataTables that you've updated a row for example could be used. Its also a bit of a performance hit taking the clone at the moment, so I'm certainly willing to put in a switch to enable and disable that operation, but I think that is probably only just the beginning of the problem with a full integration.

Anders Malmgren

Cant you expose the parts that the knockout binding needs, so that datatables does not know about knockout.

Allan Jardine

@AndersMalmgren - What are the parts the knockout bindings need beyond what the DataTables API already provides?

Lucas Martin
Owner
ducka commented May 17, 2012

Heya Allan, comments below:

The reason that DataTables is doing the clone is "bidirectional" - if it wasn't a cloned data array and you were externally to change the array - DataTables wouldn't know about that, and thus absolutely anything could go wrong - the data isn't what it is expecting. Equally, if DataTables were to modify the data source without you knowing about it, then your own processes that use the data would also likely be a bit upset.

Ok, so to be clear, I think the only modifications we really need to worry about in the datasource are the addition and deletion of items in the datasource array. When it comes to observables that are bound to cells of the table, bidirectional modifications between the two are already handled because of how I'm printing out the rows in the binding. Having said that, how do we solve the addition and deletion of rows / data source items problem...

I can't really think of a situation where I would want DataTables itself to add or remove items from it's datasource - I view adding and removing items from the data source a responsibility of the view model. I would normally create a couple of functions on the view model that add and remove items in the data source, and when the datasource mutates, the view model would notify datatables to re-render. In this regard, changes to the datasource are unidirectional (changes go from the view model to datatables, and not the other way around).

However, from what I understand about DataTables, the sorting, filtering and paging functionality requires an internally cached copy of the datasource to play with. If this is correct, I can see how removing an internally cached copy of the datasource would be a problem. Is this correct?

So the key missing element is an 'observable' variable type in Javascript (perhaps like bindable in AS3?). I know that this is one of the really attractive features of Knockout - change your parameter and the views get updated etc - however, DataTables doesn't know anything about Knockout internally, and I'd be very reluctant to make Knockout a dependency for DataTables (not because I don't like it - just because I prefer to keep dependencies to a minimum!).

I agree. Creating a dependency on Knockout JS would be a bad idea, and also unnecessary I think.

Given the overlap between Knockout and DataTables, and with no observable variables between them, I think (please anyone jump in with ideas!) cloning, and syncing the API calls between them is the only thing that can be done (delete from Knockout and delete from DataTables, or have one as the "master" which will inform the other of an action).

I believe what you mentioned in the latter is what I'm suggesting in my comments above. I think it's pretty reasonable to expect the View Model to control the data table. That's it's purpose really.

What do you think?

Anders Malmgren

I've been in contact with Allan about being able to move the rendering of the TR element down to the template. He asked me to move the discussion here so we got all the knockout / datatables discussion in one place.

I need to be able to set style on row basis (Probably a pretty standard use case) I realize that this can be a problem, datatables needs to set odd / even styles etc on the row, and probably more stuff that I dont even know about.

How about adding an option in datatables for a callback that returns the TR element, this way the knockout binding can be the creator of the TR element and return it to datatables so it can add styles etc to it.

What do you think?

Allan Jardine

It is not currently possible to have DataTables not create a TR element, but it is possible to use fnCreatedRow (called right after a row is created) to throw the row that DataTables creates away and then use the view created TR.

However, then you could end up with a situation where the TR contains data that DataTables doesn't know about - this comes back to overlapping functionality. The way I'm thinking the integration might work best is to use Knockout as the data store, but DataTables for presenting the data - including rendering the DOM elements. There wouldn't be two way data binding, so you would always need to update the data in the Knockout collection, but that is probably what I would expect anyway. Then the compatibility layer would tell DataTables about the new data and what it is.

Lucas Martin
Owner
ducka commented May 29, 2012

I believe I initially played with this idea in the binding. DataTables would create a row, and then I would replace it with my own from the row template. From what I remember however, I encountered problems with this technique because there were certain attributes/classes on the original row which datatables required, and replacing the old row with the printed template was causing DataTables some issues. You could probably copy the properties from the old row to the new row once the row template has been printed, but I'm not sure if that's going to solve the problem. I haven't looked at that issue for a while.

Regardless, we should probably keep this thread on topic. Have a play with the binding if you like @AndersMalmgren and post me a pull request if you think you have a solution.

Tanguy Krotoff

Could you please write in the README (in big) that it is not compatible with datatables 1.9.x (or specify the versions compatible).
I've spend 2 days figuring out what was going on with my app.

Thanks.

Lucas Martin
Owner

No problem tkrotoff.

Miroslav Popovic

The workaround that Allan suggested in first post works really well. Thanks! In order to avoid writing to much code for each column binding, I've wrapped the function into a helper:

ko.dataTablesProperty = function (name) {
    return function (data, type, val) {
        if (type === 'set') {
            data[name](val);
        } else {
            return data[name]();
        }
    };
};

Now I can just do: mDataProp: ko.dataTablesProperty('propertyName').

memelet

I'm a bit confused as to the status. Is there a fix that allows knockout to be used with the latest DT?

Allan Jardine

Hmmm - sort of. DataTables 1.10 development has already had the change made to it to have it use the original data source object for the row's data, rather than cloning the data - so fundamentally DataTables itself doesn't stand in the way of the integration any more. However, there are still a few things to be done before full integration can be achieved I think - primarily the new API for DataTables 1.10 which is not yet complete (indeed barely started), which will allow data to be invalidated in the table, and thus read front he source again. I think that will be required for the integration.

Anders Malmgren

You have to use 1.9 for now, also have a look at this native KO Grid binding

https://github.com/ericmbarnard/KoGrid

Alex Frenkel

Is there any news about this?

Allan Jardine

My commit from before ( #9 (comment) ) is still the current situation in DataTables core. I've not been able to progress DataTables 1.10 as quickly as I'd hoped due to contract commitments (bringing cash in :-) ), but work is still very much on going.

Allan Jardine

Bit of an update - I've just committed a number of changes into DataTables which I think should resolve this issue. I've added a rows().invalidate() method which will cause DataTables to read the information from the data source object again, and then use that latest data.

This I think is how DataTables will be able to integrate with Knockout and similar libraries. At least that's the plan. Haven't had time to test it yet, but invalidate() at least will be in 1.10!

Mason Meyer masonkmeyer referenced this issue from a commit July 31, 2013
Added some code to handle Issue #9. 27c253c
Tommy Holm Jakobsen

Any news on this?

Tommy Holm Jakobsen

Thanks. But I kinda liked this version because I think the view is more decoupled from the viewmodel. But I guess there has been no progress in getting this binding to work?

Allan Jardine

Suspect its been waiting on me with my 1.10 work, which is almost in beta now. Using observables was the only useful way I found of linking the two together, but I'm a complete novice at Knockout, so any other ideas are very welcome.

Tommy Holm Jakobsen

Sounds good with the beta. Can't wait to see the new features and documentation. I'm just learning Knockout, so my knowledge is very limited. I'll be following this thread.

Anton Romanov

the same here :) , any news?

Anton Romanov

just tested, 1.9.4 is working fine, 1.10 not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.