Skip to content

Commit

Permalink
refactor(store/persistance): ensure preloaded state is in valid shape
Browse files Browse the repository at this point in the history
  • Loading branch information
exuanbo committed Dec 11, 2023
1 parent 0d80cde commit 75d3cea
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 22 deletions.
75 changes: 75 additions & 0 deletions __tests__/common/utils/mergeSafe.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { mergeSafe } from '@/common/utils'

describe('mergeSafe', () => {
it('should not merge when types are different', () => {
const target = { a: 1 }
const source = 'string'
const result = mergeSafe(target, source)
expect(result).toEqual(target)
})

it('should deeply merge when key from source exists in target', () => {
const target = { a: { b: { c: 1 }, d: 3 } }
const source = { a: { b: { c: 2 } } }
const result = mergeSafe(target, source)
expect(result).toEqual({ a: { b: { c: 2 }, d: 3 } })
})

it('should not merge when key from source does not exist in target', () => {
const target = { a: 1 }
const source = { b: 2 }
const result = mergeSafe(target, source)
expect(result).toEqual(target)
})

it('should not deeply merge when nested key from source does not exist in target', () => {
const target = { a: { b: 1 } }
const source = { a: { c: 2 } }
const result = mergeSafe(target, source)
expect(result).toEqual(target)
})

it('should not merge arrays', () => {
const target = { a: [1, 2] }
const source = { a: [3, 4] }
const result = mergeSafe(target, source)
expect(result).toEqual(source)
})

it('should handle symbols as keys', () => {
const key = Symbol('key')
const target = { [key]: 1 }
const source = { [key]: 2 }
const result = mergeSafe(target, source)
expect(result[key]).toBe(2)
})

it('should not mutate the original objects', () => {
const target = { a: 1 }
const source = { a: 2 }
const result = mergeSafe(target, source)
expect(result).not.toBe(target)
expect(result).not.toBe(source)
})

it('should not deeply mutate the original objects', () => {
const target = { a: { b: 1 } }
const source = { a: { b: 2 } }
const result = mergeSafe(target, source)
expect(result.a).not.toBe(target.a)
expect(result.a).not.toBe(source.a)
})

it('should handle undefined and null values', () => {
const target = { a: 1, b: undefined }
const source = { b: null, c: 3 }
const result = mergeSafe(target, source)
expect(result).toEqual({ a: 1, b: undefined })
})

it('should return the target when no sources are provided', () => {
const target = { a: 1 }
const result = mergeSafe(target)
expect(result).toEqual(target)
})
})
5 changes: 2 additions & 3 deletions src/app/store/persistence/index.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { createSelector } from '@reduxjs/toolkit'
import type { Test as TypeTest } from 'ts-toolbelt'

import { ary, isPlainObject, merge, type PlainObject } from '@/common/utils'
import { ary, isPlainObject, merge, mergeSafe, type PlainObject } from '@/common/utils'
import { controllerSlice } from '@/features/controller/controllerSlice'
import { editorSlice } from '@/features/editor/editorSlice'

import { getCombinedProvider } from './combinedProvider'
import type { PersistenceProvider } from './types'

// TODO: validate the state shape
const provider = getCombinedProvider(ary(merge<PlainObject>, 2))(isPlainObject, {})

type PreloadedState = {
Expand All @@ -19,7 +18,7 @@ type PreloadedState = {
export const readStateFromPersistence = (): PreloadedState => {
const persistedState = provider.read()
// in case of future changes to the state shape
return merge(
return mergeSafe(
{
[editorSlice.reducerPath]: editorSlice.getInitialState(),
[controllerSlice.reducerPath]: controllerSlice.getInitialState(),
Expand Down
10 changes: 10 additions & 0 deletions src/common/utils/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ const parseStringRecursively = (str: string): string => {
export const stringToAscii = (str: string): number[] =>
str.split('').map((char) => char.charCodeAt(0))

const getType = (value: unknown): string => Object.prototype.toString.call(value)

export const isSameType = <T>(a: T, b: unknown): b is T => getType(a) === getType(b)

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type PlainObject = Record<string | number | symbol, any>

export const isPlainObject = (value: unknown): value is PlainObject =>
getType(value) === '[object Object]'

export const arrayShallowEqual = (a: unknown[], b: unknown[]): boolean => {
if (b.length !== a.length) {
return false
Expand Down
1 change: 1 addition & 0 deletions src/common/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export * from './classNames'
export * from './common'
export * from './merge'
export * from './mergeSafe'
export * from './types'
28 changes: 9 additions & 19 deletions src/common/utils/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@

import type { O } from 'ts-toolbelt'

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type PlainObject = Record<string | number | symbol, any>

export const isPlainObject = (value: unknown): value is PlainObject =>
Object.prototype.toString.call(value) === '[object Object]'
import { isPlainObject, type PlainObject } from './common'

const mergeRecursively = (target: unknown, source: PlainObject): PlainObject => {
const resultObject: PlainObject = {}
const result: PlainObject = {}
const sourcePropertyNames = Object.getOwnPropertyNames(source)
const sourcePropertySymbols = Object.getOwnPropertySymbols(source)
const isTargetPlainObject = isPlainObject(target)
Expand All @@ -24,9 +20,7 @@ const mergeRecursively = (target: unknown, source: PlainObject): PlainObject =>
(!isKeySymbol && !sourcePropertyNames.includes(key)) ||
(isKeySymbol && !sourcePropertySymbols.includes(key))
) {
Object.defineProperty(resultObject, key, {
...Object.getOwnPropertyDescriptor(target, key),
})
Object.defineProperty(result, key, Object.getOwnPropertyDescriptor(target, key)!)
}
}
targetPropertyNames.forEach(assignTargetProperty)
Expand All @@ -37,19 +31,17 @@ const mergeRecursively = (target: unknown, source: PlainObject): PlainObject =>
const shouldMerge = isTargetPlainObject && isPlainObject(sourcePropertyValue)
if (shouldMerge) {
const targetPropertyValue: unknown = target[key]
Object.defineProperty(resultObject, key, {
Object.defineProperty(result, key, {
...Object.getOwnPropertyDescriptor(source, key),
value: mergeRecursively(targetPropertyValue, sourcePropertyValue),
})
} else {
Object.defineProperty(resultObject, key, {
...Object.getOwnPropertyDescriptor(source, key),
})
Object.defineProperty(result, key, Object.getOwnPropertyDescriptor(source, key)!)
}
}
sourcePropertyNames.forEach(assignSourceProperty)
sourcePropertySymbols.forEach(assignSourceProperty)
return resultObject
return result
}

// https://stackoverflow.com/a/57683652/13346012
Expand All @@ -59,10 +51,8 @@ type ExpandDeep<T> = T extends Record<string | number | symbol, unknown>
? Array<ExpandDeep<E>>
: T

type Merge = <Target extends PlainObject, Sources extends PlainObject[] = Target[]>(
export const merge = <Target extends PlainObject, Sources extends PlainObject[] = Target[]>(
target: Target,
...sources: Sources
) => ExpandDeep<O.Assign<Target, Sources, 'deep'>>

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const merge: Merge = (target, ...sources) => sources.reduce<any>(mergeRecursively, target)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
): ExpandDeep<O.Assign<Target, Sources, 'deep'>> => sources.reduce<any>(mergeRecursively, target)
43 changes: 43 additions & 0 deletions src/common/utils/mergeSafe.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { isPlainObject, isSameType, type PlainObject } from './common'

const mergeSafeRecursive = <Target>(target: Target, source: unknown): Target => {
if (!isSameType(target, source)) {
return target
}
// source is guaranteed to be a plain object here but TypeScript doesn't know that
if (!isPlainObject(target) || !isPlainObject(source)) {
return source
}
const result: PlainObject = {}
const targetPropertyNames = Object.getOwnPropertyNames(target)
const targetPropertySymbols = Object.getOwnPropertySymbols(target)
const assignTargetProperty = (key: string | symbol): void => {
Object.defineProperty(result, key, Object.getOwnPropertyDescriptor(target, key)!)
}
targetPropertyNames.forEach(assignTargetProperty)
targetPropertySymbols.forEach(assignTargetProperty)
const sourcePropertyNames = Object.getOwnPropertyNames(source)
const sourcePropertySymbols = Object.getOwnPropertySymbols(source)
const assignSourceProperty = (key: string | symbol): void => {
const isKeySymbol = typeof key === 'symbol'
if (
(!isKeySymbol && targetPropertyNames.includes(key)) ||
(isKeySymbol && targetPropertySymbols.includes(key))
) {
const targetPropertyValue = result[key]
const sourcePropertyValue: unknown = source[key]
Object.defineProperty(result, key, {
...Object.getOwnPropertyDescriptor(source, key),
value: mergeSafeRecursive(targetPropertyValue, sourcePropertyValue),
})
}
}
sourcePropertyNames.forEach(assignSourceProperty)
sourcePropertySymbols.forEach(assignSourceProperty)
return result
}

export const mergeSafe = <Target, Sources extends unknown[]>(
target: Target,
...sources: Sources
): Target => sources.reduce<Target>(mergeSafeRecursive, target)

0 comments on commit 75d3cea

Please sign in to comment.