typestack / class-transformer

Decorator-based transformation, serialization, and deserialization between objects and classes.
MIT License
6.84k stars 500 forks source link

fix: plainToInstance loses values for the Map type if key matches a prototype value of Map (e.g. "size") #1342

Open haug-den-lucas opened 2 years ago

haug-den-lucas commented 2 years ago

Description

I have a class which contains a Map as an attribute:

export class Test {
    @Type(() => Number)
    public testMap: Map<string, number>;

    constructor() {
        this.testMap = new Map();
    }
}

My map contains a key-value pair, e.g. "size" -> 1337 which is preserved when calling instanceToPlain but is lost when calling plainToInstance

Minimal code-snippet showcasing the problem The following code adds some values to the map, converts it to plain and then back to the instance of Test.

import {instanceToPlain, plainToInstance, Type} from "class-transformer";
import "reflect-metadata";

const test = new Test()
test.testMap.set("something", 42)
test.testMap.set("size", 1337)
console.log("Original", test)

const plain = instanceToPlain(test)
console.log("Plain", plain)

const test2 = plainToInstance(Test, plain)
console.log("New", test2)

And my tsconfig.json

"compilerOptions": {
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "target": "ES2021",
    "module": "commonjs",
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "strict": true,
    "noImplicitAny": true,
    "alwaysStrict": true,
    "noFallthroughCasesInSwitch": true,
    "skipLibCheck": true,
    "declaration": true
  },

Expected behavior

I expect the map containing exactly the same (key, value) pairs in the test and the test2 instance.

Actual behavior

The map in test2 is missing the size value. Here is the console output of the code above:

Original: Test { testMap: Map(2) { 'something' => 42, 'size' => 1337 } }
Plain: { testMap: { something: 42, size: 1337 } }
New: Test { testMap: Map(1) { 'something' => 42 } }

The same happens for other keys which are part of the Map prototype itself, like has, clear, ...

Further analysis

I already inspected the code and found the problem, which is in this part of the code: https://github.com/typestack/class-transformer/blob/67fec03b9e32b5d020a4ee9e8ebff35e5dbfb2d7/src/TransformOperationExecutor.ts#L298-L306 When this code is called during transformation newValueKey is size and descriptor then is {get: [Function: get size],set: undefined,enumerable: false,configurable: true} which makes the expression in L303 evaluate to true and thus skips adding size to the map with the continue in L306.

While skipping these values for objects makes a lot of sense it does not for the Map type: https://github.com/typestack/class-transformer/blob/67fec03b9e32b5d020a4ee9e8ebff35e5dbfb2d7/src/TransformOperationExecutor.ts#L344-L350 In this code snippet when the value is actually added to the object or map there is a check if the type is a Map and if yes, the .set() function of Map is used, thus not inducing any problems with keys like size

Possible fix

A possible fix should be pretty easy, just replace this code https://github.com/typestack/class-transformer/blob/67fec03b9e32b5d020a4ee9e8ebff35e5dbfb2d7/src/TransformOperationExecutor.ts#L297 by

if (newValue.constructor.prototype && !(newValue instanceof Map)) {
cbjjensen commented 1 year ago

Had this issue as well. I reverted to version 0.3.1 (there may be versions in between, but I do know that 0.3.1 works for sure).