feat: use array to store paths

[AliYusuf95]

Solve #299

[AliYusuf95]

@nartc I have a note about this line:

mapper/packages/core/src/lib/map/map.ts

Line 226 in f7e89d6

while (i--) {

Is it on purpose to start in reverse order? I assumed that forMemer should overwrite the default mapping so I change it to the normal order in the PR. 😅

[nartc]

@nartc I have a note about this line:

mapper/packages/core/src/lib/map/map.ts

Line 226 in f7e89d6

while (i--) {

Is it on purpose to start in reverse order? I assumed that forMemer should overwrite the default mapping so I change it to the normal order in the PR. 😅

It is. Because in initializeMapping, I do start in reverse. forMember does override default mapping. The only affect it would have is the order of the mapped result. We can double check in the tests.

[AliYusuf95]

It is. Because in initializeMapping, I do start in reverse. forMember does override default mapping. The only affect it would have is the order of the mapped result. We can double check in the tests.

It's true that initializeMapping adds mappings in reverse order, also createMapForMember overrides the mapping for exists properties. However, if the property not in the source, then createMapForMember pushes the override property to the end.

mapper/packages/core/src/lib/create-mapper/create-map-for-member.util.ts

Line 80 in f7e89d6

mapping[MappingClassId.properties].push([memberPath, mappingProperty]);

My expectation was that when I use forMember in the parent class, this should override the result for the inner properties. That's why I changed the loop to the normal order.

// example test
export const deepNestedFooFooFooProfile: MappingProfile = (mapper) => {
  mapper.createMap(FooFooFoo, FooFooFooDto).forMember(
    (dest) => dest.foo,
    fromValue('FooFooFoo Custom Value')
  );
  mapper.createMap(FooFoo, FooFooDto).forMember(
    (dest) => dest.foo.foo,
    fromValue('FooFoo Custom Value')
  );
  mapper.createMap(Foo, FooDto).forMember(
    (dest) => dest.foo.foo.foo,
    fromValue('Foo Custom Value')
  );
};

// Succeed
    it('should map forMember override properly', () => {
      mapper.addProfile(deepNestedFooFooFooProfile);

      const foo = new FooFooFoo();
      foo.foo = 'some value';

      const vm = mapper.map(foo, FooFooFooDto, FooFooFoo);
      expect(vm).toBeTruthy();
      expect(vm.foo).toEqual('FooFooFoo Custom Value');
    });

// Failed
    it('should map inner forMember override properly', () => {
      mapper.addProfile(deepNestedFooFooFooProfile);

      const foo = new FooFoo();
      foo.foo = new FooFooFoo();
      foo.foo.foo = 'some value';

      const vm = mapper.map(foo, FooFooDto, FooFoo);
      expect(vm).toBeTruthy();
      expect(vm.foo.foo).toEqual('FooFoo Custom Value');  // <= Received: "FooFooFoo Custom Value"
    });

// Failed
    it('should map deep inner forMember override properly', () => {
      mapper.addProfile(deepNestedFooFooFooProfile);

      const vm = mapper.map({ foo: {foo: { foo: 'some value' }} }, FooDto, Foo);
      expect(vm).toBeTruthy();
      expect(vm.foo.foo.foo).toEqual('Foo Custom Value'); // <= Received: "FooFooFoo Custom Value"
    });

I'm not sure if this behavior is intended or not, but in the previous version, I was able to override the inner properties.

[nartc]

That is definitely a bug imho. If we change from mapping[MappingClassId.properties].push([memberPath, mappingProperty]); to mapping[MappingClassId.properties].unshift([memberPath, mappingProperty]);, what do you think? performance wise? If anything, I'd prefer to make the code of mapping configurations (that likely to run once, like forMember()) less performant than to make the code that map().

[AliYusuf95]

It could be unshift, as you said more likely it'll run only once. But I didn't get the point of the process:

• Adding initial properties in reversed order to the map
• unshift new properties
• Start the mapping in the reversed order

It could be just the normal order with push in this case.

[nartc]

I’m fine with push + normal order as well. We can go with that. If that’s the case, we can actually convert all reverse loops to normal loops?

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
8 Code Smells

No Coverage information
0.0% Duplication

[AliYusuf95]

I have changed other reversed loops to normal loops 👍

[nartc]

Thanks man. I'll be reviewing this today!

[nartc]

Thanks a lot for the PR. LGTM. It actually cleans up quite a bit.