Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ko.toJS() serialize every property including ordinal functions #251

Closed
eldargab opened this issue Dec 28, 2011 · 11 comments
Closed

ko.toJS() serialize every property including ordinal functions #251

eldargab opened this issue Dec 28, 2011 · 11 comments
Milestone

Comments

@eldargab
Copy link

Hi. I stumbled across funny case while serializing my model and passing it to jQuery.ajax().
The following code suddenly caused stack overflow error (after clicking "save" button).

 <button data-bind="click: save">Save</button>
function AdvertModel () {
    this.name = ko.observable();
    this.description = ko.observable();

    var self = this;

    this.save = function (data, evt) {
        var advert = ko.toJS(self);
        $.post('/new-advert', advert, function () {
            self.clear();
        });
    }

    this.clear = function () {
        this.name(null);
        this.description(null);
    }
}

That's because after serializing AdvertModel it's save method was included into result, at the same time jQuery's serialization method treats all functions as getters and invokes them. As a result jQuery calls model's save() method again and we have recursion.

Personally, I would excluded functions automatically (just without thinking). They have no sens as data and the only reason for ko.toJS is to extract data. But I don't insist - "remove functions immediately!". I just wanted to share this issue.

@pierol
Copy link

pierol commented Apr 10, 2012

I think i've a similar problem. My view model is this
viewModel = {
init: function () {
$.ajax({
type: 'GET',
url: 'admincategoria/init',
async: false,
contentType: 'application/json; charset=utf-8',
success: function (data) {
$.extend(viewModel, ko.mapping.fromJS(data));
},
error: function () {

            }
        });
    },
    vuoto: function () {
        $.ajax({
            type: 'GET',
            url: 'admincategoria/init',
            async: false,
            contentType: 'application/json; charset=utf-8',
            success: function (data) {
                ko.mapping.fromJS(data, viewModel)
            },
            error: function () {

            }
        });
    },
    save: function () {
        $.ajax({
            type: 'POST',
            url: 'admincategoria/update',
            contentType: 'application/json; charset=utf-8',
            /*data: JSON.stringify(ko.mapping.toJS(viewModel)),*/
            data: ko.mapping.toJSON(viewModel),
            success: function (data) {
                $('#modal-categoria').modal('hide');
                $treeCategorie.jstree("refresh");
            },
            error: function (jqXHR, textStatus, errorThrown) { alert('Errore:' + errorThrown) }
        });
    },
    edit: function (key) {
        $.ajax({
            type: 'GET',
            async: false,
            url: 'admincategoria/detail?IdCategoria=' + key,
            contentType: 'application/json; charset=utf-8',
            success: function (data) {
                ko.mapping.fromJS(data, viewModel);
                $('#modal-title').text("Modifica Categoria");
                $('#modal-categoria').modal('show');
            },
            error: function (jqXHR, textStatus, errorThrown) { alert('Errore:' + errorThrown) }
        });
    },
    erase: function () {
        $.ajax({
            type: 'POST',
            url: 'admincategoria/delete',
            async: false,
            data: JSON.stringify({ IdCategoria: viewModel.Categoria.IdCategoria() }),
            contentType: 'application/json; charset=utf-8',
            success: function (data) {
                $("#modal-delete-categoria").modal('hide');
                $treeCategorie.jstree("refresh");

            },
            error: function (jqXHR, textStatus, errorThrown) { alert('Errore:' + errorThrown) }
        });
    },
    removeImmagine: function (url, data) {
        viewModel.Immagini.remove(function (item) {
            return (item.Url == url)
        });
        return true;
    }

}

what happens is that after five o six call to the save method of the viewmodel the call toJSON (i've tried with toJS same issue) cause a stack overflow with version 2.0.0 of knockout (in chrome says thant the page is crashed, in firefox that a script doesn't respond anymore). With the last source of knockout nothing happens and the interface doesn't respond to the save action.

@ntoshev
Copy link

ntoshev commented Jul 25, 2012

+1

Examples in http://knockoutjs.com/documentation/json-data.html lead you right in this trap :)

@ScottIngram
Copy link

I just ran into tis problem too. Because of it, I can't use jQuery.ajax(data:ko.toJS). I must instead use jQuery.ajax(data:{"json_field":ko.toJSON}), and then decode json_field on the server side.

@mbest
Copy link
Member

mbest commented Aug 1, 2012

Agree this should be fixed. Maybe we can get it in 2.2.

@rniemeyer
Copy link
Member

We could make ko.toJS exclude functions. I would not imagine that many would be relying on it keeping functions.

Just wanted to mention that I have never had a problem with $.ajax when specifying it like:

$.ajax({
    type: "POST",
    url: url,
    data: ko.toJSON(data),
    contentType: "application/json"
}

So, we use ko.toJSON and specify that the content type is json.

@ScottIngram
Copy link

http://jsfiddle.net/singram/H23WX/

This jsFiddle demonstrates how the functions are invoked by $.ajax -- notice how the list of aliases gets a new, blank entry on each save (unintended functionality). The Person object includes a method to add new aliases to itself. Seems like reasonable OO design. But $.ajax() misappropriates that function and triggers it during the conversion.

Sorry for the sloppy example. I didn't pare it down to just the relevant code, but I tried to arrange the important bits to the top.

Also, I updated it to be as close as possible to Ryan's example above. The difference being that I use ko.toJS and he uses ko.toJSON which avoids this problem. However, ko.toJSON causes more work on the server side in that the fields are not automatically parsed into a map by the server, but instead, is a flat string that I must parse as JSON into a map. I'm using Grails, btw.

@rastasheep
Copy link

+1 on this !
i use var object = JSON.parse(ko.toJSON(data));

@paglias paglias mentioned this issue Feb 7, 2013
@paglias paglias mentioned this issue Mar 6, 2013
22 tasks
@jdiaz5513
Copy link

I was bit by this bug too. The problem partially lies with the documentation - I expected the behavior to not include member functions but this was not clearly stated in the docs so I had to make an assumption. 3cf90eb should definitely be merged but the documentation should also clearly state that member functions are not included when calling ko.toJS (and ko.toJSON, for that matter).

@uberllama
Copy link

Hey guys, was this ever committed? Its causing me all kinds of headaches currently. Cheers.

@mbest
Copy link
Member

mbest commented Mar 3, 2014

@uberllama It was, but then reverted. See #1079.

@uberllama
Copy link

Thanks for the info. I'll transition my code to use toJSON. Cheers.

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

Successfully merging a pull request may close this issue.

9 participants