knockout / knockout

Knockout makes it easier to create rich, responsive UIs with JavaScript
http://knockoutjs.com/
Other
10.43k stars 1.52k forks source link

Inconsistent behavior of ko.toJS in ES5 vs ES6+ for objects with toJSON method #2585

Closed Aaron-P closed 2 years ago

Aaron-P commented 2 years ago

If a class is created using ES5 'prototype' syntax and a toJSON method is defined on the prototype, that method gets carried through the ko.toJS (and thus ko.toJSON) method. For example:

var Test1 = (function () {
    function Test1() {
    }
    Test1.prototype.toJSON = function () {
        return {
            a: 1337
        };
    };
    return Test1;
}());

var test1 = new Test1();
console.log(ko.toJS(test1).toJSON());

Returns:

{a: 1337}

If ES6+ class syntax is used with a toJSON method, it is not carried through. For example:

class Test2 {
    constructor() {
    }
    toJSON() {
        return {
            b: 666
        };
    }
}

var test2 = new Test2();
console.log(ko.toJS(test2).toJSON());

Throws:

Uncaught TypeError: ko.toJS(...).toJSON is not a function
    at <anonymous>:1:28

The issue seems to be in the method visitPropertiesOrArrayEntries where if toJSON is defined on the prototype it is picked up by the "for (var propertyName in rootObject)" loop, but if it's defined as a method on a class definition it does not. An explicit check for rootObject["toJSON"] may resolve the direct issue, but there may still be some additional problems if the toJSON is expecting other methods to be carried through as well and they are also missed by the "for in" loop. I'm not sure if there's a better way of handle that.

mbest commented 2 years ago

Class methods are non-enumerable by default.

image

If you want a specific method to be enumerable you can override the enumerable setting:

class Test2 {
    constructor() {
    }
    toJSON() {
        return {
            b: 666
        };
    }
}
Object.defineProperty(Test2.prototype, 'toJSON', {enumerable: true});

var test2 = new Test2();