Skip to content

fix(core) merge function does not handle all cases correctly #13030

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 34 additions & 15 deletions packages/core/src/lib/utils/merge.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
function isObject(item: unknown): item is object {
return item !== null && typeof item === "object"
function isPlainObject(item: unknown): item is Record<string, unknown> {
return (
item !== null &&
typeof item === "object" &&
!Array.isArray(item) &&
!(item instanceof Date) &&
!(item instanceof RegExp) &&
!(item instanceof Error) &&
Object.prototype.toString.call(item) === "[object Object]"
)
}

/** Deep merge two or more objects */
Expand All @@ -8,23 +16,34 @@ export function merge<T extends Record<string, unknown>>(
...sources: Array<Record<string, unknown> | undefined>
): T & Record<string, unknown> {
if (!sources.length) return target
const source = sources.shift()

if (isObject(target) && isObject(source)) {
for (const key in source) {
if (isObject(source[key])) {
if (!isObject(target[key]))
(target as Record<string, unknown>)[key] = Array.isArray(source[key])
? []
: {}
for (const source of sources) {
if (!source) {
continue
}

// Use Object.keys to avoid prototype pollution
for (const key of Object.keys(source)) {
const sourceValue = source[key]
const targetValue = (target as Record<string, unknown>)[key]

// Skip undefined values from source
if (sourceValue === undefined) {
continue
}

// If both values are plain objects, merge them recursively
if (isPlainObject(targetValue) && isPlainObject(sourceValue)) {
merge(
(target as Record<string, unknown>)[key] as T,
source[key] as Record<string, unknown>
targetValue as Record<string, unknown>,
sourceValue as Record<string, unknown>
)
} else if (source[key] !== undefined)
(target as Record<string, unknown>)[key] = source[key]
} else {
// Otherwise, override the target value with the source value
;(target as Record<string, unknown>)[key] = sourceValue
}
}
}

return merge(target, ...sources)
return target as T & Record<string, unknown>
}
236 changes: 236 additions & 0 deletions packages/core/test/merge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,240 @@ describe("merge function", () => {
authorization: "https://example.com/user-config",
})
})

it("should handle merging Date object", () => {
const target = {
sessionToken: {
name: "authjs.session-token",
options: {
httpOnly: true,
sameSite: "lax",
path: "/",
secure: false,
},
},
}
const source = {
sessionToken: {
options: {
expires: new Date("2024-01-01T00:00:00Z"),
},
},
}

const expected = {
sessionToken: {
name: "authjs.session-token",
options: {
httpOnly: true,
sameSite: "lax",
path: "/",
secure: false,
expires: source.sessionToken.options.expires,
},
},
}

expect(merge(target, source)).toEqual(expected)
})

it("should handle RegExp objects", () => {
const target = { pattern: /old/g }
const source = { pattern: /new/i }

const result = merge(target, source)
expect(result.pattern).toEqual(/new/i)
expect(result.pattern.flags).toBe("i")
})

it("should handle Error objects", () => {
const target = { error: new Error("old error") }
const source = { error: new TypeError("new error") }

const result = merge(target, source)
expect(result.error).toBeInstanceOf(TypeError)
expect(result.error.message).toBe("new error")
})

it("should handle mixed types correctly", () => {
const target = {
number: 1,
string: "hello",
boolean: true,
array: [1, 2, 3],
object: { nested: "value" },
nullValue: null,
undefinedValue: undefined,
}

const source = {
number: 2,
string: "world",
boolean: false,
array: [4, 5],
object: { nested: "new value", added: "property" },
nullValue: "not null anymore",
undefinedValue: "defined now",
newProperty: "brand new",
}

const result = merge(target, source)

expect(result).toEqual({
number: 2,
string: "world",
boolean: false,
array: [4, 5],
object: { nested: "new value", added: "property" },
nullValue: "not null anymore",
undefinedValue: "defined now",
newProperty: "brand new",
})
})

it("should mutate the target object", () => {
const target = { a: 1, b: { c: 2 } }
const source = { b: { d: 3 } }
const originalTarget = JSON.parse(JSON.stringify(target))
const originalSource = JSON.parse(JSON.stringify(source))

const result = merge(target, source)

// Target should be mutated
expect(target).not.toEqual(originalTarget)
expect(target).toBe(result) // Should return the same reference
expect(target.b.c).toBe(2) // Original nested properties preserved
expect((target.b as any).d).toBe(3) // New properties added

// Source should not be mutated
expect(source).toEqual(originalSource)
})

it("should handle deeply nested objects", () => {
const target = {
level1: {
level2: {
level3: {
level4: {
value: "original",
},
},
},
},
}

const source = {
level1: {
level2: {
level3: {
level4: {
value: "updated",
newValue: "added",
},
newLevel4: "added",
},
newLevel3: "added",
},
newLevel2: "added",
},
newLevel1: "added",
}

const result = merge(target, source)

expect(result).toEqual({
level1: {
level2: {
level3: {
level4: {
value: "updated",
newValue: "added",
},
newLevel4: "added",
},
newLevel3: "added",
},
newLevel2: "added",
},
newLevel1: "added",
})
})

it("should handle class instances correctly", () => {
class TestClass {
constructor(public value: string) {}
}

const target = { instance: new TestClass("original") }
const source = { instance: new TestClass("updated") }

const result = merge(target, source)
expect(result.instance).toBeInstanceOf(TestClass)
expect(result.instance.value).toBe("updated")
})

it("should handle symbols as values", () => {
const sym1 = Symbol("test1")
const sym2 = Symbol("test2")

const target = { symbol: sym1 }
const source = { symbol: sym2 }

const result = merge(target, source)
expect(result.symbol).toBe(sym2)
})

it("should handle arrays with objects", () => {
const target = { items: [{ id: 1, name: "item1" }] }
const source = {
items: [
{ id: 2, name: "item2" },
{ id: 3, name: "item3" },
],
}

const result = merge(target, source)
expect(result.items).toEqual([
{ id: 2, name: "item2" },
{ id: 3, name: "item3" },
])
})

it("should handle empty arrays", () => {
const target = { items: [1, 2, 3] }
const source = { items: [] }

const result = merge(target, source)
expect(result.items).toEqual([])
})

it("should handle constructor property", () => {
const target = { constructor: Object }
const source = { constructor: Array }

const result = merge(target, source)
expect(result.constructor).toBe(Array)
})

it("should handle Map and Set objects", () => {
const map1 = new Map([["key1", "value1"]])
const map2 = new Map([["key2", "value2"]])
const set1 = new Set([1, 2])
const set2 = new Set([3, 4])

const target = { map: map1, set: set1 }
const source = { map: map2, set: set2 }

const result = merge(target, source)
expect(result.map).toBe(map2)
expect(result.set).toBe(set2)
})

it("should handle objects with numeric keys", () => {
const target = { 0: "zero", 1: "one" }
const source = { 1: "updated one", 2: "two" }

const result = merge(target, source)
expect(result).toEqual({ 0: "zero", 1: "updated one", 2: "two" })
})
})