fix: devalue will return private data to the client even if toJSON is implemented.

[CleverLili]

nuxt-contrib/devalue does not handle toJSON properly. There was a fix made earlier and there is an open pull request for another fix. However, that pull request is not correct either.

Here is an example that demonstrates the problems. There are 2 problems:

• The current implementation would not run toJSON always and instead return the object with all its hidden properties.
• If the same object is referenced twice, only the first one is properly handled. For example, let's say you have an array of 2 products and each product is referencing the same User object instance. The existing code will call toJSON on the first product1.User but not the product2.User. Again, it returns the hidden properties.
• The existing code will not handle arrays of objects that also need to be toJSONified.

Here is example: There is a class User that implements toJSON where it excludes certain fields. In this case, it's "internal" that should be excluded. In the example below, you will see that "old code output" shows internal returned to the client.

class User
{
    constructor()
    {
        this.valueA = 1;
        this.valueB = 2;
        this.internal = 3;
    }

    toJSON()
    {
        //internal is removed.
        return {valueA: this.valueA, valueB: this.valueB}
    }
}


let user = new User();

let data = {
    data       : user,
    arrayOfData: [user, new User()],
};

console.warn('[TEST1] The output below shows the property "internal" when it was removed in the toJSON.')
console.log('current code output: ', devalue(data));
console.log('new code output    : ', "(function(a){a.valueA=1;a.valueB=2;return {data:a,arrayOfData:[a,{valueA:1,valueB:2}]}}({}))");
console.log('old code output    : ', "(function(a){a.valueA=1;a.valueB=2;a.internal=3;return {data:a,arrayOfData:[a,{valueA:1,valueB:2}]}}({}))");

I tried very hard to not touch the existing code to make sure that nothing breaks. So you'll see that I let the existing code add a property to a map and then I remove it. I also introduced additional walk parameters to handle the update of the parent with the JSONified data. I would like to rewrite the code at some point but I have to deal with duplicates which this code doesn't handle perfectly when dealing with JSONified objects. There is a performance concern with that so I have a hack that I'm happy to discuss with the team once we get past this.

I ran the mocha test and all 61 are passing.

My motivation

Other than this is a security issue as it may return data that isn't meant for the browser. I'm returning my object data and hydrating it on the client with the proper class instance. Without toJSON working properly I cannot return the class name that needs to be instantiated. This fix will solve both the security problem and give programmers a way to hydrate their own data.

In my 30 years of programming, this is the first time I make edits to a Typescript file so let me know if I could do things better.

[manniL]

cc @pi0

[CleverLili]

@manniL @pi0 sorry for the stupid question but it seems I had lost a fix when I tried to do the pull request (I had trouble locally). Do I need to cancel this pull and create a new one?

[pi0]

Hi @CleverLili. Sorry for late reply. Sure it seems better idea also aligning with latest main refactors.