Skip undefine destination value when map mutate

[huybn5776]

For PATCH api, I only want to take the not null/undefined field from source to destination but not update all missing fields to undefined.

const user = {
  firstName: 'foo',
  lastName: 'bar',
} as User;
const userDto = {
  lastName: 'what',
} as UserDto;

mapper.map(userDto, User, UserDto, user);

console.log(user);
// expected tobe { firstName: 'foo', lastName: 'what' } but { firstName: undefined, lastName: 'what' } 

This may implementation by add an predicateFn to setMember() of the map.ts, and add an option to enable this feature.

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

const setMember = (valFn: () => unknown, predicateFn?: (destinationFieldValue: unknown) => boolean) => {
  try {
    const val = valFn();
    if (!predicateFn || predicateFn?.(val)) {
      setMemberFn(destinationFieldValue, destination)(val);
      return;
    }
  } catch (originalError) {
    // ...
  }
  // ...
  setMember(
    () =>
      mapMember(
        transformationMapFn,
        sourceObj,
        destination,
        destinationMemberPath,
        extraArguments,
        mapper,
      ),
    (destinationFieldValue) => destinationFieldValue !== undefined,
  )
}

I know this may be an unreasonable feature for AutoMapper, that it should map all fields from source to destination, but just to make patch process more simple.

[nartc]

Hi, thanks for reporting the issue. That seems like a bug for sure. And it seems like this is only applicable to mapMutate which means a predicateFn isn't necessarily needed in this case. We can adjust this directly for mapMutate https://github.com/nartc/mapper/blob/main/packages/core/src/lib/map/map.ts#L173 to check destinationFieldValue before running setMutate().

export function mapMutate<
  TSource extends Dictionary<TSource> = any,
  TDestination extends Dictionary<TDestination> = any
>(
  sourceObj: TSource,
  mapping: Mapping<TSource, TDestination>,
  options: MapOptions<TSource, TDestination>,
  mapper: Mapper,
  errorHandler: ErrorHandler,
  destinationObj: TDestination
): void {
  map(
    sourceObj,
    mapping,
    options,
    mapper,
    errorHandler,
    (destinationMember: string[]) => (value: unknown) => {
      // logic to check for undefined
      // this function has access to 
      // - value: the value on SourceObject
      // - sourceObj: the sourceObject itself
      // - destinationObj: the destinationObject itself
      setMutate(destinationObj, destinationMember, value);
    }
  );
}

Does it make sense? Would you like to submit a PR?

[huybn5776]

Is that a bug? I tried .NET version of AutoMapper and it give me the same result, that is mapping null value to destination of same property name.

Or, it maybe a bug on js version of AutoMapper because it's not null but undefined, then we can directly modify the behavior without add option to enable it. I will take over this part and make a PR.

[nartc]

@huybn5776 I've never used map mutably in .NET before so I'm not aware of its behavior. If that's the case, what would you do in .NET?

In early versions of this library, I did have it map to null for empty values but it didn’t really play nice with JS so I just map undefined now, or basically “whatevet the default value that the JS engine interprets”

[huybn5776]

Still for PATCH request, this is the behavior of .NET version of AutoMapper.

public class User
{
    public string FirstName { get; set; } = default!;
    public string LastName { get; set; } = default!;
}
    
public class UserDto
{
    public string? FirstName { get; set; }
    public string? LastName { get; set; }
}

var user = new User
{
    FirstName = "foo",
    LastName = "bar"
};
var userDto = new UserDto
{
    FirstName = "Kung"
};
_mapper.Map<UserDto, User>(userDto, user);
// user = { FirstName = "Kung" }

In order to patch value, i write another method on my c# project to do this without using AutoMapper because it's hard to find out how to only patch non-null property when mapping with AutoMapper.

And for js version of AutoMapper, i think we can make it better without to do some workaround when someone need it.

[nartc]

@huybn5776 Yes. I think the proposed solution up there would be good to have as a default aka to ignore the value if the value is falsy**. In the future, if other devs need to configure that, we can always add an option later. Please feel free to go ahead and submit a PR if you can. Thank you!

**: About this point, I think we should keep it as undefined for now and leave null as is.

const user = {
  firstName: 'foo',
  lastName: 'bar',
} as User;
const userDto = {
  lastName: 'what',
} as UserDto;

mapper.map(userDto, User, UserDto, user);

console.log(user); // should log User { firstName: 'foo', lastName: 'what' }

const user = {
  firstName: 'foo',
  lastName: 'bar',
} as User;
const userDto = {
  firstName: null,
  lastName: 'what',
} as UserDto;

mapper.map(userDto, User, UserDto, user);

console.log(user); // should log User { firstName: null, lastName: 'what' }

Since the userDto#firstName can be explicitly set to null in some cases for PATCH.

[huybn5776]

@nartc Did you expected to also apply patch for nested object?

const user: User = {
  firstName: 'foo',
  profile: {
    bio: 'Introvert-ish',
    birthday: new Date('10/14/1991'),
  },
};
const userVm: UserVm = {
  profile: {
    bio: 'Outgoing-ish',
  },
};

mapper.map(userVm, User, UserVm, user);
console.log(user);

// current behavior: 
// { firstName: 'foo', profile: { bio: 'Outgoing-ish' } }
// nested patch:
// { firstName: 'foo', profile: { bio: 'Outgoing-ish', birthday: new Date('10/14/1991') } }

[nartc]

I guess we need to consider this for nested object as well.