Skip to content
This repository was archived by the owner on Feb 2, 2025. It is now read-only.

Add the possibility to call DTInstances.getList() and DTInstances.getLast() wherever you are #234

Closed
l-lin opened this issue Mar 2, 2015 · 22 comments
Milestone

Comments

@l-lin
Copy link
Owner

l-lin commented Mar 2, 2015

At the moment, when calling after the table is rendered, the promise returned by DTInstances.getList() and DTinstances.getLast() are not resolved.
One example in this pen.

The issue comes from the fact thoses promises are resolved when the datatables instances (instances generated by DataTables) are registered. Except that the promises are created when calling DTInstances.getList() or DTInstances.getLast(). So the promises are never resolved.
In short:

  • Normal way in sequential time: DTInstances.getList() => Create promise defered => DTInstances.register() => Resolve the promise
  • What really happen: DTInstances.register() => Tyring to resolve an inexisting promise => DTInstances.getList() => Create promise defered
@miguelqvd
Copy link
Contributor

Hello,

First of all thank you for your great job with Angular-DataTables. I'm using it since few weeks and I found issues when I have several tables at the same time under the same controller or different ones when trying to call getList()

I debugged it and I found that getList uses a promise that relies at the same time in another promise
_deferLastDTInstances. I've modified the code so we create juste one promise in function register(dtInstance, result) and then forced getList just to return such promise. I'm probably missing something to understand the original implementation but since I have this running everything seems to work great.

Thanks a lot.

Cheers

Miguel-Ángel

function register(dtInstance, result) {
    dtInstance.id = result.id;
    dtInstance.DataTable = result.DataTable;
    dtInstance.dataTable = result.dataTable;

    _instances[dtInstance.id] = dtInstance;

    if (_deferLastDTInstances === null) {
        _deferLastDTInstances = $q.defer();
    }

    _deferLastDTInstances.resolve(dtInstance);


    if (_deferDTInstances === null) {
        _deferDTInstances = $q.defer();
    }

    _deferDTInstances.resolve(_instances);

    return dtInstance;
}

function getLast() {
    return _deferLastDTInstances.promise;
}

function getList() {
    return _deferDTInstances.promise;
}

@l-lin
Copy link
Owner Author

l-lin commented Mar 7, 2015

Thanks for your suggestion. However, the issue with your solution is that it always use the same promise. As you know, a promise, once resolved, cannot be resolved with another value.
So if you change in another page in your SPA that contains another DataTable, the latter will not be registered in the promise. Thus, the DTInstances.getList() and DTInstances.getLast() will not return the latest DataTables instance.

@miguelqvd
Copy link
Contributor

Thank you for your answer. To be honest I'm working with Angular JS since not so long and I'm familiar with promises.

I took your remarks and I did this modifications which also works fine.

How does this look like?

/* @ngInject */
function dtInstances($q) {
var _instances = {};
var _lastInstance = {};

return {
    register: register,
    getLast: getLast,
    getList: getList
};

function register(dtInstance, result) {
    dtInstance.id = result.id;
    dtInstance.DataTable = result.DataTable;
    dtInstance.dataTable = result.dataTable;

    _instances[dtInstance.id] = dtInstance;
    _lastInstance = dtInstance;

    return dtInstance;
}

function getLast() {

    var getLastPromise = $q.defer();

    getLastPromise.resolve(_instances);

    return getLastPromise.promise;
}

function getList() {
    var getListPromise = $q.defer();

    getListPromise.resolve(_instances);

    return getListPromise.promise;
}

}

@l-lin
Copy link
Owner Author

l-lin commented Mar 7, 2015

That won't do either because if you try to get the instances or the last instance before the datatable is registered, then they will both return an empty object.
Thanks for your suggestion though.

@ajoslin103
Copy link

I too am having issues with DTInstances.getList()

I call it when a table is clicked on and it fails

TypeError: Cannot read property 'then' of null
at Object.getList (angular-datatables.js:457)
at controllerFn.vm.setHiliteById (tables-settings.js:35)
at controllerFn.vm.rowClicked (tables-settings.js:23)
at tables-settings.js:54
at Scope.$get.Scope.$eval (angular.js:14401)
at Scope.$get.Scope.$apply (angular.js:14500)
at HTMLTableCellElement. (tables-settings.js:53)
at HTMLTableCellElement.jQuery.event.dispatch (jquery.js:4430)
at HTMLTableCellElement.jQuery.event.add.elemData.handle (jquery.js:4116)

@ajoslin103
Copy link

I spent a few hours mucking about with this and ended up adding and exposing a DTInstances.returnList(){ return _instances; } function -- that gives me a workaround for the broken getList() for now.

I think that getLast and getList could certainly be blocked with a single promise that ensures that at least one instance has been registered and added to the list.

But after that you can no longer block on additional instances because there is no way to know if there will ever be any more tables registered. I suspect that getList() and getLast() can not be implemented after the first instance is registered.

I suggest a getInstance promise requiring a tableId parameter as a way to determine which instance the user is looking for -- and blocking until the instance with that tableId is registered.

And a getList promise that ensures that at least one instance has been registered and added to the list.

getLast is not useful to me - I have 3 tables on a page and using getLast in each of their controllers, but I get the 1st one 3 times because the controllers start up before the DOM is realized and the 1st one to register resolves for all of them.

@miguelqvd
Copy link
Contributor

Hi, I'm probably missing the global picture but I would like to understand why should someone expect something else than an empty object or even an empty array when any datatable hasn't been still registered. Wouldn't be that case something to be handle by the developer?

l-lin commented a day ago

That won't do either because if you try to get the instances or the last instance before the datatable is registered, then they will both return an empty object.
Thanks for your suggestion though.

PS: I'm very interested in contributing to this issue as it's already a core component of my solution.

@l-lin
Copy link
Owner Author

l-lin commented Mar 8, 2015

That means you won't be able to fetch the datatable instances even after the tables are rendered.
In short, in sequential order:

  • You fetcht the DTInstances => it returns an empty object
  • Your tables are rendered => DTInstances still returns an empty object whereas we want the DT instances.

@miguelqvd
Copy link
Contributor

Thanks for your clarification. I've created this plunker with those two case of uses:

http://plnkr.co/edit/I1BEU6lIdFkAqxxwGadf?p=preview

When you load the app DTInstances is invoked and a simple list is filled then we can click on a button and fetch DTInstances again.

Sorry if I don't understand your point or if I'm still missing something.

@l-lin
Copy link
Owner Author

l-lin commented Mar 8, 2015

See your edited plnkr. I just changed displaying the instances from angular to the console.
You will notice that the first call in the console does not display the list.
Why is it showing when displayed in the $scope? It must be related to angular digest cycle as $q is also using it.

@ajoslin103
Copy link

I submitted a pull request with the code simplified to only promise the first instance, it was the only way I could make getList() work for more than one usage. Let me know if you figure out how to promise additional instances, I sure would like to see how that works !!

@poisa
Copy link

poisa commented Mar 11, 2015

I would love to find a solution to this problem too as I can only get the instance right after I create the table but never again. Given that I'm working on a CMS and that I will need to add and remove tables very often, not being able to reload the table data poses a bit of a problem for me!

@miguelqvd
Copy link
Contributor

I'm working on it. By the time being I've managed to have reload functionality running sacrificing the access to DTInstances on page load but the having full access to all the tables when I need it any user interface event:

var refreshItemsTable = function () {
    $scope.selectedItem = null;

    DTInstances.getList().then(function (dtInstances) {
        dtInstances.itemsTable.reloadData();
    });
};

I mean, with the last solution that I proposed and which was incompleted as l-lin showed me in plunkr.

@miguelqvd
Copy link
Contributor

I think I found the solution or at least almost there. Please check my plunkr:
http://plnkr.co/edit/I1BEU6lIdFkAqxxwGadf

The issue that I have now us that Angular doesn't apply the results to the user view although the object contains the data expected.

We can see in the console that console.log shows us that the tables were found.

@miguelqvd
Copy link
Contributor

Yes!! :-) It's working!

@miguelqvd
Copy link
Contributor

I've created a pull request.

@l-lin
Copy link
Owner Author

l-lin commented Mar 11, 2015

Resolved in the PR #242.

@l-lin l-lin closed this as completed Mar 11, 2015
@poisa
Copy link

poisa commented Mar 12, 2015

This is great! After installing the latest dev branch with this patch I can finally get the instance using getLast(). My problem now is that the table will not reload it's data at all. What am I doing wrong?

    $scope.submit = function()
    {
        $scope.member.put().then(function(member)
        {
            $scope.editFormIsVisible = false;
            $scope.tableIsVisible = true;

            DTInstances.getLast().then(function(dtInstance)
            {
                console.debug('got instance', dtInstance);
                dtInstance.reloadData();
            });
        });
    };

This is the dump I see in Chrome. It does look like there is a function reloadData() but nothing is happening and the table shows the old data until I reload the whole page.

dump

@l-lin
Copy link
Owner Author

l-lin commented Mar 12, 2015

What renderer are you using? Check this pen. The request is resent to the server when dtInstance.reloadData() is called.

If you are using the Angular renderer, there are no reloadData (check this code). You have to do it manually. It shouldn't be difficult with the two way databinding.

PS: I see you are using a promise inside a promise. You can write better code by chaining promises:

$scope.member.put().then(function(member) {
    $scope.editFormIsVisible = false;
    $scope.tableIsVisible = true;
    return DTInstances.getLast();
}).then(function(dtInstance) {
    console.debug('got instance', dtInstance);
    dtInstance.reloadData();
});

@poisa
Copy link

poisa commented Mar 12, 2015

Regarding the chained promises, thanks for the tip. I deeply dislike unnecessarily nested code and having never worked with promises before I didn't know how to solve that!

I don't know exactly what's the name of the renderer I'm using. In fact I couldn't figure out how to get a reference to the data the instance was using so I had to "intercept" it and store it in my scope.

    $scope.tableData = null;

    function serverData(sSource, aoData, fnCallback, oSettings)
    {
        $scope.edit = edit;
        $scope.delete = deleteRow;

        // Inject table name into request 
        aoData.table = 'members';

        oSettings.jqXHR = $.ajax({
            'dataType': 'json',
            'type': 'POST',
            'url': 'http://tsp.dev/datatables',
            'data': aoData,
            'cache': false,
            'success': function(json)
            {
                $scope.$apply(function()
                {
                    // Store a copy of the data in my scope to prevent 
                    // an extra server roundtrip when editing the row
                    $scope.tableData = json;
                });

                fnCallback(json);
            }
        });
    }

    $scope.dtOptions = DTOptionsBuilder.newOptions()
        .withOption('serverSide', true)
        .withPaginationType('full_numbers')
        .withOption('createdRow', createdRow)
        .withFnServerData(serverData);

    $scope.dtColumns = [
        DTColumnBuilder.newColumn('id').withTitle('id'),
        DTColumnBuilder.newColumn('name').withTitle('Name'),
        DTColumnBuilder.newColumn('email').withTitle('Email'),
        DTColumnBuilder.newColumn('birthdate').withTitle('Birthdate'),
        DTColumnBuilder.newColumn(null).withTitle('Actions').notSortable().renderWith(actionsHtml)
    ];

(I had to do all that unfortunate oSettings.jqXHR business because I need to inject an extra post param so that the backend knows which MySQL table to return data from)

@gjlloyd
Copy link

gjlloyd commented Mar 13, 2015

i'm using v0.4.2 and am still getting the undefined dtInstances object. the only difference that I see vs the working plunkers is that we are configuring the tables via the dtOptions/dtBuilders instead of just hard coded static tables. we originally upgraded to 0.4 in hopes of resolving the dreaded 'datatables can't reinitialze tables' error, which keeps popping up sporadically in our app. i've read through all these posts but still don't know what to do. we have debugged into the getList() function and can see that my tables are instantiated correctly, but the promise just never gets resolved.

@miguelqvd
Copy link
Contributor

Hi,

Thank you for your feeback. I'm using also DTOptions/DTBuilders on my project and everything is running fine. I've also thought about the solution given and it I don't guess the reason why this is not working for you. Could you please provide a plunkr with an example so we can debug it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants