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: npm linking from a package that uses class-transformer breaks decorators #1043

Open
cpmoser opened this issue Dec 20, 2021 · 14 comments
Open
Labels
status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature.

Comments

@cpmoser
Copy link

cpmoser commented Dec 20, 2021

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.

@cpmoser cpmoser added status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature. labels Dec 20, 2021
@cpmoser cpmoser changed the title fix: <your-title-goes-here> fix: npm linking from a package that uses class-transformer breaks decorators Dec 20, 2021
@loadko
Copy link

loadko commented Jan 4, 2022

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

Hi!

I have the same issue.

Is someone have a workaround?

Thanks,

@WangHansen
Copy link

WangHansen commented Feb 11, 2022

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:

  • The share package needs to have reflect-metadata on it needs to be in the entry file
  • Use version 0.4.0 of the class-transformer
  • Make sure to delete package-lock.json after downgrade(I spent so long to figure it out)

Then it starts to work for me

@jonasschultheiss
Copy link

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

fzamperin commented Jun 1, 2022

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

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

@Quovadisqc
Copy link

Quovadisqc commented Sep 7, 2022

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

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

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

Leejjon commented Dec 31, 2022

Never mind, my issue was caused by something different.

@huybuidac
Copy link

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

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

cadmax commented Oct 30, 2023

{
'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.

@MagicFame
Copy link

MagicFame commented Jan 17, 2025

Nestjs changed the necessary functions in version 11 :

export interface TransformerPackage {
  plainToInstance<T>(
    cls: Type<T>,
    plain: unknown,
    options?: ClassTransformOptions,
  ): T | T[];
  classToPlain(
    object: unknown,
    options?: ClassTransformOptions,
  ): Record<string, any> | Record<string, any>[];
}

You should now put this instead :

import { ClassTransformer } from 'class-transformer';

const transformer = new ClassTransformer();

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

export { transformerPackage };

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.