Skip to content

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

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

Open
haug-den-lucas opened this issue Sep 16, 2022 · 1 comment
Labels
status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature.

Comments

@haug-den-lucas
Copy link

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:

const descriptor = Object.getOwnPropertyDescriptor(newValue.constructor.prototype, newValueKey);
if (
(this.transformationType === TransformationType.PLAIN_TO_CLASS ||
this.transformationType === TransformationType.CLASS_TO_CLASS) &&
// eslint-disable-next-line @typescript-eslint/unbound-method
((descriptor && !descriptor.set) || newValue[newValueKey] instanceof Function)
)
// || TransformationType === TransformationType.CLASS_TO_CLASS
continue;

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:

if (finalValue !== undefined || this.options.exposeUnsetFields) {
if (newValue instanceof Map) {
newValue.set(newValueKey, finalValue);
} else {
newValue[newValueKey] = finalValue;
}
}

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

if (newValue.constructor.prototype) {

by

if (newValue.constructor.prototype && !(newValue instanceof Map)) {
@haug-den-lucas haug-den-lucas added status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature. labels Sep 16, 2022
haug-den-lucas added a commit to haug-den-lucas/class-transformer that referenced this issue Sep 16, 2022
@cbjjensen
Copy link

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature.
Development

No branches or pull requests

2 participants