typestack / class-transformer

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

fix: npm linking from a package that uses class-transformer breaks decorators #1043

Open cpmoser opened 2 years ago

cpmoser commented 2 years ago

Description

Using @Type, @Transform and @Expose decorators in a linked package (npm link in development environment) and extending classes from the linked package nullifies the decorators on the extended class. This probably applies to other decorators as well.

class-transformer is a peer dependency of my linked package. The linked package also includes the class-transformer in its node_modules (so I can npm run build:watch). The issue arises due to the fact that the metadata storage won't be shared between the two (my app and my linked package) - a workaround is to npm link class-transformer from my app and then npm link class-transformer from my linked package but this is a little hacky and probably fragile.

class-validator uses the global mechanism for metadata storage and I would propose using that for class-transformer as well. See https://github.com/typestack/class-validator/blob/4e39a04fd966782aa8ee7f2d56bdbdb05c956469/src/metadata/MetadataStorage.ts#L144-L152

Minimal code-snippet showcasing the problem

Linked npm package:

export class BaseClass {
  @Expose()
  someProperty;
}

Main app:

import { BaseClass } from 'linked-package';

export class ExtendedClass extends BaseClass { }

const a = new ExtendedClass();
a.someProperty = 'not actually exposed';

console.log(plainToClass(ExtendedClass, a)); // results in '{}'

Expected behavior

Expected someProperty to be included in the plainToClass transformation.

Actual behavior

someProperty is not copied over to the new instance of ExtendedClass.

loadko commented 2 years ago

Hi,

Take a look at this issue. I've added your class to my package index.ts and in my tsc-app, index.ts I've added:

import { ..., BaseClass } from 'tsc-package/dist'

export class ExtendedClass extends BaseClass { }

const a = new ExtendedClass()
a.someProperty = 'not actually exposed'

console.log(plainToClass(ExtendedClass, a)) // results in ExtendedClass { someProperty: 'not actually exposed' }

Take a look at my setup and compare, for example, I didn't install the package with link. I made npm install ../tsc-package

EDIT: I've installed package with link and I get the same result: ExtendedClass { someProperty: 'not actually exposed' }

pinonpierre commented 2 years ago

Hi!

I have the same issue.

Is someone have a workaround?

Thanks,

WangHansen commented 2 years ago

I have a similar setup where I put all the DTOs in a shared folder and use npm workspace to manage the monorepo where frontend and backend uses the shared folder as dependency.

I was able to get things working with following steps:

Then it starts to work for me

jonasschultheiss commented 2 years ago

i face the same issue. we're currently building a nestjs backend with microservices. we have to have the same dto's in the service and the gateway. this is blocking our current implementation from validating number inputs in queries.

i'd gladly help with a pr, but i have no experience with oss and don't know how this package works. if this project is actively maintained, i urge the maintainers to fix this, as the above mentioned workaround doesn't work for me.

fzamperin commented 2 years ago

I'm having the same issue, I'm using packages inside a monorepo, although @WangHansen workaround works for me, the problem is that I'm using nestjs, and version 0.4.0 breaks with it

narrowizard commented 2 years ago

Same issue here.
And I find that defaultMetadataStorage is an individual component. But there is no way to replace it.

Quovadisqc commented 2 years ago

I resolved my issue by doing the following;

When linking an npm package from my project, I add the following to my project's webpack config:

resolve: {
        alias: {
            'class-transformer': path.resolve('./node_modules/class-transformer'),
            'reflect-metadata': path.resolve('./node_modules/reflect-metadata'), // Or the decorators polyfill that you use
        },
    },

This ensures that my library resolves the same instances than my project for both these packages and fixes the decorator issues.

The issue seem to be caused by the fact that when linking a library, I develop the library locally and have a node_modules folder. From my project build, the library still resolves packages from the library's node_modules which seem to cause issues with class-transformer.

code4break commented 1 year ago

@jonasschultheiss I was faced the same situation as you. I could solve it by setting all class-transformer installations (lib and microservices) to the exactly same version. If you do so, npm will collapse them into one folder (node_modules/class-transformer) instead of multiple installation in different subfolders (eg node_modules/my-lib/transformer, node_modules/my-service/transformer). It works because the metadata of the decorators are stored in this folder. If you have multiple class-transformer folders within the node_module and its subdirectories, it can't find the right one.

ReneZeidler commented 1 year ago

I encountered the same issue in a different context. @nestjs/mapped-types (which is used by @nestjs/swagger) implements decorators like PickType, PartialType, etc. which have to be used to make the Swagger module work.

As part of these types, they copy class-transformer and class-validation metadata from one class to another. To do this, they require('class-transformer/cjs/storage') here.

In my own code, I import decorators directly from class-transformer. This works fine with tsc, but when I use webpack to bundle the application, suddenly the transformation stops working correctly. I traced this issue back to the fact that the import of the storage module in @nestjs/mapped-types resolves to a different module than the storage module used by my own code's class-transformer import. The result of this is that all derived types using PickType (etc.) don't copy the class-transformer metadata.

I don't know why exactly the modules resolve the way they do, and why tsc behaves differently than webpack, but it's probably a combination of the weird hacky import of class-transformer/cjs/storage in @nestjs/mapped-types and the fact that the metadata storage of class-transformer isn't designed to be imported directly by other libraries.

class-validator doesn't have this problem because it provides a getMetadataStorage() method that @nestjs/mapped-types uses.


Anyways, if anyone else encounters the same problem, I managed to find a workaround using @Quovadisqc's comment above with some adjustments. I added this to resolve.alias in my webpack.config file:

{
  'class-transformer/cjs/storage': path.resolve('./node_modules/class-transformer/cjs/storage'),
  'class-transformer': path.resolve('./node_modules/class-transformer/cjs'),
}

This forces all imports of class-transformer to resolve to the same implementation and therefore the same storage.

Leejjon commented 1 year ago

Never mind, my issue was caused by something different.

huybuidac commented 1 year ago
{
  'class-transformer/cjs/storage': path.resolve('./node_modules/class-transformer/cjs/storage'),
  'class-transformer': path.resolve('./node_modules/class-transformer/cjs'),
}

you saved 2 days of my life.

MartinDrost commented 1 year ago

After going through the suggested solutions in this thread, I decided not to use the webpack approach to resolve the issue. Instead, I exposed a custom transformerPackage from the shared package folder and used it in the ValidationPipe in the Nestjs project.

In the transformer-package.ts in the shared package folder

import { ClassTransformer } from 'class-transformer';

const transformer = new ClassTransformer();

// map classToPlain and plainToClass since they are renamed in class-validator
// but Nestjs still expects them.
const transformerPackage = {
  classToPlain: transformer.instanceToPlain,
  plainToClass: transformer.plainToInstance,
};

export { transformerPackage };

In the main.ts of the Nestjs project

import { transformerPackage } from 'shared-folder';

...
app.useGlobalPipes(
  new ValidationPipe({
    transform: true,
    transformerPackage,
  }),
);
...

This approach prevents you from having to tinker with the build steps of Nestjs and preserves the metadata of the used class-transformer instances.

cadmax commented 10 months ago

{ 'class-transformer/cjs/storage': path.resolve('./node_modules/class-transformer/cjs/storage'), 'class-transformer': path.resolve('./node_modules/class-transformer/cjs'), }

you saved 3 days of my life.