Skip to content
Merged
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
27 changes: 24 additions & 3 deletions packages/router-core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,18 @@ export function replaceEqualDeep<T>(prev: any, _next: T): T {

const array = isPlainArray(prev) && isPlainArray(next)

if (array || (isPlainObject(prev) && isPlainObject(next))) {
const prevItems = array ? prev : Object.keys(prev)
if (array || (isSimplePlainObject(prev) && isSimplePlainObject(next))) {
const prevItems = array
? prev
: (Object.keys(prev) as Array<unknown>).concat(
Object.getOwnPropertySymbols(prev),
)
const prevSize = prevItems.length
const nextItems = array ? next : Object.keys(next)
const nextItems = array
? next
: (Object.keys(next) as Array<unknown>).concat(
Object.getOwnPropertySymbols(next),
)
const nextSize = nextItems.length
const copy: any = array ? [] : {}

Expand Down Expand Up @@ -245,6 +253,19 @@ export function replaceEqualDeep<T>(prev: any, _next: T): T {
return next
}

/**
* A wrapper around `isPlainObject` with additional checks to ensure that it is not
* only a plain object, but also one that is "clone-friendly" (doesn't have any
* non-enumerable properties).
*/
function isSimplePlainObject(o: any) {
return (
// all the checks from isPlainObject are more likely to hit so we perform them first
isPlainObject(o) &&
Object.getOwnPropertyNames(o).length === Object.keys(o).length
)
}

// Copied from: https://github.com/jonschlinkert/is-plain-object
export function isPlainObject(o: any) {
if (!hasObjectPrototype(o)) {
Expand Down
38 changes: 38 additions & 0 deletions packages/router-core/tests/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,44 @@ describe('replaceEqualDeep', () => {
expect(result).toStrictEqual(obj2)
})

describe('symbol properties', () => {
it('should look at symbol properties in the object comparison', () => {
const propertyKey = Symbol('property')
const obj1 = { a: 1, [propertyKey]: 2 }
const obj2 = { a: 1, [propertyKey]: 3 }
const result = replaceEqualDeep(obj1, obj2)
expect(result).toStrictEqual(obj2)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without this PR:

expect(result).toBe(obj1)

})

it('should copy over symbol properties when creating a new object', () => {
const propertyKey = Symbol('property')
const obj1 = { a: 1, [propertyKey]: 2 }
const obj2 = { a: 3, [propertyKey]: 2 }
const result = replaceEqualDeep(obj1, obj2)
expect(result).toStrictEqual(obj2)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without this PR:

expect(result).toStrictEqual({ a: 3 })

})
})

describe('non-enumerable properties', () => {
it('should treat objects with non-enumerable properties as non-plain (no need for property comparisons)', () => {
const obj1: { a: number; b?: number } = { a: 1 }
Object.defineProperty(obj1, 'b', { enumerable: false, value: 2 })
const obj2: { a: number; b?: number } = { a: 1 }
Object.defineProperty(obj2, 'b', { enumerable: false, value: 3 })
const result = replaceEqualDeep(obj1, obj2)
expect(result).toBe(obj2)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without this PR:

expect(result).toBe(obj1)
expect(result.b).toBe(2)

})

it("should treat objects with non-enumerable properties as non-plain (copying doesn't happen)", () => {
const obj1: { a: number; b?: number } = { a: 1 }
Object.defineProperty(obj1, 'b', { enumerable: false, value: 2 })
const obj2: { a: number; b?: number } = { a: 3 }
Object.defineProperty(obj2, 'b', { enumerable: false, value: 2 })
const result = replaceEqualDeep(obj1, obj2)
expect(result).toBe(obj2)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without this PR:

expect(result).toStrictEqual({ a: 3 })
expect(result.b).toBe(undefined)

})
})

it('should properly handle non-existent keys', () => {
const obj1 = { a: 2, c: 123 }
const obj2 = { a: 2, c: 123, b: undefined }
Expand Down