Breaking change in 2.0.3 for override fields

[vsamofal]

Did you read the migration guide?

• I have read the whole migration guide

Is there an existing issue that is already proposing this?

• I have searched the existing issues

Potential Commit/PR that introduced the regression

c65d434

Versions

2.0.0 -> 2.0.3

Describe the regression

sample entities hierarchy:

abstract class BaseEntityHelper {
        @IsUUID()
        @Expose()
        id!: string | number;
        
        @Expose()
        anotherField!: string;
    }

    class SampleEntity extends BaseEntityHelper {

        @Expose()
        @IsUUID()
        override id!: string;

        @Expose()
        @IsString()
        name!: string;

    }

    class SampleEntityDto extends OmitType(SampleEntity, ['name'] as const) {

    }

So we have an abstract class with an id that can be a number or string overriding it and exposing it. Using omit type to create DTO, that has just a name field.

Let's implement a simple test:

it('instance to instance', () => {


            let sampleEntityWithoutId = new SampleEntity();

            sampleEntityWithoutId.id = 'just id';
            sampleEntityWithoutId.name = 'name';
            sampleEntityWithoutId.anotherField = 'anotherField';


            let dto = plainToInstance(SampleEntityDto, sampleEntityWithoutId, {
              excludeExtraneousValues: true,
              exposeDefaultValues: true,
              ignoreDecorators: true,
              exposeUnsetFields: false,
            });

            let justCopy = plainToInstance(SampleEntity, sampleEntityWithoutId, {
                excludeExtraneousValues: true,
                exposeDefaultValues: true,
                ignoreDecorators: true,
                exposeUnsetFields: false,
            });

            // this is failing but should be ok
            expect(dto.anotherField).toEqual('anotherField');
        })

The error is handled over here

but thrown over here

this behavior recently changed, and it's a breaking change for this case at least.

I'm ok to fix it by myself, just some comments from maintainers on expected behavior will be good to hear

as for me we should apply the most closest decorators or probably merge it somehow, but not sure is it that obvious to do the merge or no

Minimum reproduction code

No response

Input code

provided

Expected behavior

need suggestion

Other

No response

[vsamofal]

@KuanWenChen probably you can take a look, I understand that you were fixing something, but we also break some, so need to come up with some plan for all known cases to work

[KuanWenChen]

I thing problem is happen at value is not a iterable type. I am not sure why value is not a iterable type.
I thing this could fix it:

[existingRules, targetMetadataEntries].forEach((entries) => {
  for (const [valueKey, value] of entries) {
    const arrayValue = Array.isArray(value) ? value : [value];
    if (mergeMap.has(valueKey)) {
      mergeMap.get(valueKey)!.push(...arrayValue);
    } else {
      mergeMap.set(valueKey, arrayValue);
    }
  }
});

[kamilmysliwiec]

Could you create a PR for this issue?

[KuanWenChen]

OK, I think I found the problem.
_excludeMetadatas, _exposeMetadatas, _typeMetadatas should not be a Array
So, they should be overwriten by children.
I would create PR as soon as.

[kamilmysliwiec]

Let's track this here #1225