customMerge never being called

[xenoterracide]

export function guessJodaOptions(): deepmerge.Options {
  return {
    customMerge: (key: string): ((left: unknown, right: unknown) => unknown) | undefined => {
      if (/.*(date|time|On).*/.test(key)) return jodaMerge;
      return undefined;
    },
  };
}

usage

    const defaults: UserEntityOpt = {
      username: shortUniqueString(),
      email: `${uniqueString()}@briggo.com`,
      firstName: shortUniqueString(),
      lastName: shortUniqueString(),
      password: 'DontUseThisPassword',
      isStaff: false,
      isActive: true,
      isSuperUser: false,
      dateJoined: ZonedDateTime.now(),
    };
 return repo.save(UserEntity.of(deepmerge(defaults, part, guessJodaOptions())));

note: without joda this thing works perfectly

I breakpointed it, and guessJodaOptions() is called but never customMerge.

[superguineapig]

Hi, I'm not affiliated with this library, but I find it quite useful and stumbled across similar issues recently, so I created a codesandbox to test this issue.

It seems that there are 2 problems:

• ZonedDateTime has a very large inheritance chain. That causes the merge routine to exceed the JS recursion limit. This can be avoided by setting a custom isMergeableObject that will return false for class-based objects. but...
• when isMergableObject returns false, the customMerge function will never be run. This behavior seems slightly ambiguous as the non-mergeable-object is still copied (by reference) to the output.

I think it would make sense that a custom merger should always be invoked, if available, when the item is non-mergeable.

It also might make sense to have an additional configuration option(s) to determine whether or not an object is included/excluded from the output. This would make the semantics behind isMergeableObject more "pure".

There are several scenarios to consider, all of which would change the existing behavior and API surface area. For example:

• what is the order of the pipeline? E.g. "is mergeable" -> "include/exclude" -> "custom merger"
• include/exclude semantics: E.g. "include/exclude if not-mergeable," "always copy by reference if not mergeable and no custom merger available, else exclude" etc.

I'm not sure how much the author(s) would be comfortable with changing though.

I'm happy to review and discuss as time permits.