class-transformer icon indicating copy to clipboard operation
class-transformer copied to clipboard

fix: made metadata global to prevent issues with multiple package installations

Open jtimmons opened this issue 2 years ago • 5 comments

Description

The MetadataStorage object currently acts as a singleton within the module by using the defaultMetadataStorage constant. However, this doesn't account for the scenario in which there are multiple instances of the class-transformer library installed; this seems to be common in two cases:

  1. Shared libraries using class-transformer as a prod dependency. If the library and its consumer are on two different versions of class-validator then we end up with a MetadataStorage object for each version installed
  2. Shared libraries which are symlinked (common in monorepos using workspaces, lerna, etc). Regardless of version, if there's no dep hoisting happening you'll have an instance installed in each package

Having duplicate metadata stores can have some really subtle behavior for users. In our case we were using class-transformer alongside class-validator to validate incoming messages; because our validators were coming in from a shared library the @Type() decorator was silently failing its lookup, resulting in all of our nested validators failing to run.

We were made aware of this issue by the breaking change in [email protected] (changelog) to forbid unknown values by default. Because our objects were silently being only partially transformed to DTOs by this bug, they were being considered as an "unknown value". I imagine that this may be affecting other users as well since there's been a decent amount of chatter about this on the NestJS discord and issue boards (examples: typestack/class-validator#1873, nestjs/nest#10683)

This implementation is lifted from what was done in typestack/class-validator#265 to use a global store to avoid these conflicts.

Checklist

  • [x] the pull request title describes what this PR does (not a vague title like Update index.md)
  • [x] the pull request targets the default branch of the repository (develop)
  • [x] the code follows the established code style of the repository
    • npm run prettier:check passes
    • npm run lint:check passes
  • [n/a] tests are added for the changes I made (if any source code was modified)
  • [n/a] documentation added or updated
  • [x] I have run the project locally and verified that there are no errors

Fixes

fixes #1043, fixes #928

Update: found PR #929 from 2021 which is similar but contains a lot of automatic dependency updates. This change might be easier to merge at this point so going to leave it up for one of the maintainers to decide between

jtimmons avatar Jan 24 '23 17:01 jtimmons

This is still not fixed. I'm using class-transformer class-validator class-sanitizier and routing-controllers in different packages of my project. And decorators just don't exist at all from one project to the other.

y0gzah avatar Feb 01 '23 19:02 y0gzah

This is still not fixed. I'm using class-transformer class-validator class-sanitizier and routing-controllers in different packages of my project. And decorators just don't exist at all from one project to the other.

hey @y0gzah! Do you mean that this is not fixed by this pull request when testing it? Or that the bug still exists in the library?

jtimmons avatar Feb 01 '23 22:02 jtimmons

ping pong!

nocive avatar Feb 16 '23 07:02 nocive

Running into this issue today. Similar to @jtimmons, I'm attempting to use a shared library of class-validator models between our frontend (Vue/Vite) repo and our backend (Nest.js) repo. Would love to see this prioritized.

dabernathy89 avatar Mar 23 '23 22:03 dabernathy89

+1

tetofonta avatar Dec 30 '23 17:12 tetofonta

Fixed in https://github.com/typestack/class-transformer/pull/929.

NoNameProvided avatar May 04 '24 11:05 NoNameProvided