Fix Immutable-merge for arrays

[JasonVMo]

This fixes the immutable merge code to handle arrays correctly for standard handling (which means arrays overwrite) while still allowing them to be processed by custom handlers.

This change addresses Issue #9

Custom handlers now don't have array values filtered by type. This allows merging of arbitrary values with the same keys.

The signature of the handler is now aligned with immutableMerge itself as no one was using the key parameter so it was just dead weight.

[tkamireh]

I'm still a little confused by the processSingles property on IMergeOptions. Are clients, in this case theming and styling code, supposed to call immutableMerge on a single object if they want to run custom handlers? Maybe makes sense to separate the functionality out into an immutableProcessHandlers or something.

[JasonVMo]

It is confusing but convenient. Let's say we want to run this with a custom style handler that creates the css classes and replaces the objects with strings that reference the classes. In the case where it is already a class it should not be changed. The styles in this case are deep in the tree and you want to ensure you make the minimal mutation to the tree.

A helper function isn't a bad idea. It might actually be good to create wrapping functions for common scenarios. I would maybe suggest moving the functions as follow:

immutableMerge() -> immutableMergeCore
immutableMerge(...objs[]) // just do a simple recursive immutable merge
immutableTransfform(handlers: { }, ...objs[]);  // guarantee full processing on the objects

I'm open to suggestions here. The code should be as tight as possible (for bundle size) but usage could be improved.

[JasonVMo]

Addressed in latest iteration, feedback welcome.

[tkamireh]

Do we plan to eventually add support for top level merging of arrays with a handler being passed in? Something like

immutableArrayMerge(handler: IArrayMerger, targets: any[])

This feels like it may be useful given that styles can be provided as arrays of style objects and that we may want to do merging on before letting the framework (e.g. React Native) process.

[JasonVMo]

This sounds like a good idea. The main concern is having code in a really core bundle that is not being used. We want this as small as possible. It's like I want to publish the equivalent of a header only C++ library, might be worth talking to David about philosophy here.

[PPatBoyd]

    }

I'm interested in better understanding how undefined values are processed. Am I understanding correctly that:

1. Any default merges with key->undefinedObject as a result of Object.assign(...), which would only occur when at least one object has said key mapped to undefined and no object has said key mapped to a real value, has said key deleted from the final merge object.

2. Custom handlers are allowed to merge real values to undefined, resulting in key deletion that wouldn't happen with the default merge -- what's the use case around this behavior?

It appears that Case (1) can be short-circuited and avoid the if (nextOptions) {...} logic, unless custom handlers are allowed to map keys->undefinedObjects to real values? if (2) is also true I understand leaving the key deletion to after, but would still skip the recursing logic if possible

---

Refers to: packages/immutable-merge/src/Merge.ts:79 in 1d0d6bf. [](commit_id = 1d0d6bf, deletion_comment = False)

[tkamireh]

IMergeRecursionHandler [](start = 12, length = 22)

Seems like this could benefit from a type parameter T.

[JasonVMo]

This was changed to any to allow custom aggregations of things like styles. In web they might have a signature of something like:

// this is pseudo code just to communicate the idea
type IComplexStyle = string | ICSSStyle;
type IStyle = IComplexStyle | IStyle[];

The any type basically says that you are responsible for checking and handling the types that you are handed.

[tkamireh]

immutableMerge [](start = 16, length = 14)

What are the use cases where the objects are not of the same type/interface. I suspect all this could be type parameterized.

[JasonVMo]

We could type parameterize the easy helper functions. For the core function it doesn't really work in practice because of the deep objects. I found that trying to extend complex typing to core layers has the following problems:

• it is misleading in a way because the core layers are treating things as objects
• recursion rules can have unpredictable inferences
• It causes type errors that are incredibly hard to diagnose at the endpoints. By this I mean that the core code will work but trying to use the types at the endpoints are prone to failure. So you can have two objects that are really the same type will require that one be typecast to be accepted as an input because of differences in the inference rules.
• This is similar to the approach for the component library. Make it more generic internally and let the types be defined at the endpoints.

I do like the idea of having something like:

function immutableMerge<T extends object>(...objs: T[]): T {
  return immutableMergeCore({ depth: -1 }, ...objs);
}