Skip to content
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

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

Open
tmtron opened this issue Oct 6, 2021 · 3 comments · May be fixed by #1118
Open

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

tmtron opened this issue Oct 6, 2021 · 3 comments · May be fixed by #1118
Labels
status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature.

Comments

@tmtron
Copy link
Contributor

tmtron commented Oct 6, 2021

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>'
@tmtron tmtron added status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature. labels Oct 6, 2021
@kol-93 kol-93 linked a pull request Feb 21, 2022 that will close this issue
6 tasks
@Hum4n01d
Copy link

Hum4n01d commented Apr 24, 2023

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
Copy link

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
Copy link

KeyboardDanni commented Apr 23, 2024

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.

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

Successfully merging a pull request may close this issue.

4 participants