-
Notifications
You must be signed in to change notification settings - Fork 669
Description
Is your feature request related to a problem? Please describe.
When I first tried the deepMerge function, I expected the default behavior to be as follows:
import { deepMerge } from "@std/collections/deep-merge";
import { assertEquals } from "@std/assert";
const a = { foo: [1, 2] };
const b = { foo: [2, 3] };
const result = deepMerge(a, b, { arrays: "merge" });
const expected = { foo: [1, 2, 3] };
assertEquals(result, expected);Then I realized that the implementation uses Arrays.prototype.concat() just like the default strategy by npm:deepmerge.
Describe the solution you'd like
Adding a "union" MergingStrategy for arrays only, preferably through a new exported type ArraysMergingStrategy.
Implementation can use the union function imported from "./union.ts".
Describe alternatives you've considered
Warning
The following solution would be a BREAKING CHANGE
Splitting the MergingStrategy type into 3 separate types:
export type ArraysMergingStrategy = "concat" | "union" | "replace";
export type MapsMergingStrategy = "union" | "replace";
export type SetsMergingStrategy = "union" | "replace";This way, it is clear what to expect from the strategy. Implementations of maps and sets do not need to change, just the name of the strategy (from "merge" to "union").
The default strategy should be "replace" for all, since the current fallback if the user entered an unmapped strategy, is to return right. Example below:
import { deepMerge } from "@std/collections/deep-merge";
import { assertEquals } from "@std/assert";
const a = { foo: new Set(["foo"]) };
const b = { foo: new Set(["bar"]) };
// @ts-expect-error: user passes unmapped strategy
const result = deepMerge(a, b, { sets: "unmapped-strategy" });
const expected = { foo: new Set(["bar"]) };
assertEquals(result, expected);