typestack / class-transformer

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

fix: instanceToPlain fails when union discriminator type is read-only #944

Open tmtron opened 3 years ago

tmtron commented 3 years ago

Description

When we use immutable helper libraries, like immer, the discriminator property on the instance may be frozen.
In this case instanceToPlain (in class-transformer 0.4.0) fails with:

TypeError: Cannot assign to read only property '__type' of object '#<Sports>'

Minimal code-snippet showcasing the problem

import 'reflect-metadata';
import { instanceToPlain, Type } from '../../src';
import { defaultMetadataStorage } from '../../src/storage';

it('should transform a frozen union to plain', () => {
  defaultMetadataStorage.clear();

  abstract class Hobby {
    public name: string;
  }

  class Sports extends Hobby {
    readonly __type = 'sports';
  }

  class Relaxing extends Hobby {
    readonly __type = 'relax';
  }

  class Person {
    name: string;

    @Type(() => Hobby, {
      discriminator: {
        property: '__type',
        subTypes: [
          { value: Sports, name: 'sports' },
          { value: Relaxing, name: 'relax' },
        ],
      },
    })
    hobby: Hobby;
  }

  const model = new Person();
  model.name = 'John Doe';
  const sport = new Sports();
  sport.name = 'Football';
  model.hobby = Object.freeze(sport);

  const frozenModel = model;
  const json: any = instanceToPlain(frozenModel);
  expect(json).not.toBeInstanceOf(Person);
  expect(json.hobby).toStrictEqual({
    __type: 'sports',
    name: 'Football',
  });
});

Expected behavior

Converting to plain should not fail (even for frozen instances).

Actual behavior

instanceToPlain (in class-transformer 0.4.0) fails with:

TypeError: Cannot assign to read only property '__type' of object '#<Sports>'
Hum4n01d commented 1 year ago

Bump. This is an issue when using the immer library in particular. I am getting

TypeError: Cannot add property __type, object is not extensible

If I add a placeholder __type field to my class, I run into the same issue @tmtron encountered with

TypeError: Cannot assign to read only property '__type'
kimayoi72 commented 9 months ago

I have the same problem using a discriminator in the @Type decorator on freezed instances. It seems to be, that instanceToPlain has a side-effect, modifying the instance.

My expectation is, that the discriminator property is in the plain object, but the call will never every modify the instance.

KeyboardDanni commented 6 months ago

Ran into this during a refactor. Vite didn't like the third party NPM package with the fix, and the official package isn't patch-package friendly. I ended up working around the issue by using a draft function and calling instanceToPlain() on the draft:

produce(item, (draft) => {
  json = JSON.stringify(instanceToPlain(draft));
});

It feels... wrong, but seems to work fine. Because the draft allows writing, we don't get an error at runtime. The side effects are contained within the draft instance, and we discard the resulting object. Would be better if instanceToPlain() didn't modify the source object though.