Skip to content

Commit

Permalink
refactor(ui/conf): better handling of mutable config (#722)
Browse files Browse the repository at this point in the history
* fix(ui): mergeDeep causes some configurations to be overridden by the default configuration

* fix: config always get override by defaults

Signed-off-by: Frost Ming <[email protected]>

* refactor(ui/conf): better handling of mutable config

* chore: fix path issue on win env

* fix: mergeDeep avoid prototype pollution

BREAKING CHANGE

Directly referencing `artalk.conf` and `artalk.$root` is deprecated. Please use functions instead: `artalk.getConf()` and `artalk.getEl()`.

---------

Signed-off-by: Frost Ming <[email protected]>
Co-authored-by: syfxlin <[email protected]>
Co-authored-by: Frost Ming <[email protected]>
  • Loading branch information
3 people authored Dec 26, 2023
1 parent 32a3986 commit 4c2646e
Show file tree
Hide file tree
Showing 13 changed files with 331 additions and 50 deletions.
31 changes: 24 additions & 7 deletions ui/artalk/src/artalk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,17 @@ const GlobalPlugins: ArtalkPlugin[] = [ ...DefaultPlugins ]
* @see https://artalk.js.org
*/
export default class Artalk {
public conf!: ArtalkConfig
public ctx!: ContextApi
public $root!: HTMLElement

/** Plugins */
protected plugins: ArtalkPlugin[] = [ ...GlobalPlugins ]

constructor(conf: Partial<ArtalkConfig>) {
// Init Config
this.conf = handelCustomConf(conf)
if (this.conf.el instanceof HTMLElement) this.$root = this.conf.el
const handledConf = handelCustomConf(conf, true)

// Init Context
this.ctx = new Context(this.conf, this.$root)
this.ctx = new Context(handledConf)

// Init Services
Object.entries(Services).forEach(([name, initService]) => {
Expand All @@ -49,6 +46,16 @@ export default class Artalk {
this.ctx.trigger('inited')
}

/** Get the config of Artalk */
public getConf() {
return this.ctx.getConf()
}

/** Get the root element of Artalk */
public getEl() {
return this.ctx.$root
}

/** Update config of Artalk */
public update(conf: Partial<ArtalkConfig>) {
this.ctx.updateConf(conf)
Expand All @@ -63,7 +70,7 @@ export default class Artalk {
/** Destroy instance of Artalk */
public destroy() {
this.ctx.trigger('destroy')
this.$root.remove()
this.ctx.$root.remove()
}

/** Add an event listener */
Expand Down Expand Up @@ -103,7 +110,7 @@ export default class Artalk {

/** Load count widget */
public static loadCountWidget(c: Partial<ArtalkConfig>) {
const conf = handelCustomConf(c)
const conf = handelCustomConf(c, true)

Stat.initCountWidget({
getApi: () => new Api(convertApiOptions(conf)),
Expand All @@ -113,4 +120,14 @@ export default class Artalk {
pvAdd: false
})
}

// ===========================
// Deprecated
// ===========================

/** @deprecated Please use `getEl()` instead */
public get $root() { return this.ctx.$root }

/** @description Please use `getConf()` instead */
public get conf() { return this.ctx.getConf() }
}
18 changes: 10 additions & 8 deletions ui/artalk/src/config.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
import type { ArtalkConfig, ContextApi } from '~/types'
import type { ApiOptions } from './api/_options'
import * as Utils from './lib/utils'
import { mergeDeep } from './lib/merge-deep'
import Defaults from './defaults'


/**
* Merges the user custom config with the default config
* Handle the custom config which is provided by the user
*
* @param customConf - The custom config object which is provided by the user
* @returns The config for Artalk instance creation
*/
export function handelCustomConf(customConf: Partial<ArtalkConfig>): ArtalkConfig {
export function handelCustomConf(customConf: Partial<ArtalkConfig>, mergeDefault: true): ArtalkConfig
export function handelCustomConf(customConf: Partial<ArtalkConfig>, mergeDefault?: false): Partial<ArtalkConfig>
export function handelCustomConf(customConf: Partial<ArtalkConfig>, mergeDefault = false) {
// 合并默认配置
const conf: ArtalkConfig = Utils.mergeDeep({ ...Defaults }, customConf)

// TODO the type of el options may HTMLElement, use it directly instead of from mergeDeep
if (customConf.el) conf.el = customConf.el
const conf: Partial<ArtalkConfig> = mergeDefault ? mergeDeep(Defaults, customConf) : customConf

// 绑定元素
if (typeof conf.el === 'string' && !!conf.el) {
Expand All @@ -29,7 +29,9 @@ export function handelCustomConf(customConf: Partial<ArtalkConfig>): ArtalkConfi
}

// 服务器配置
conf.server = conf.server.replace(/\/$/, '').replace(/\/api\/?$/, '')
if (conf.server) {
conf.server = conf.server.replace(/\/$/, '').replace(/\/api\/?$/, '')
}

// 默认 pageKey
if (!conf.pageKey) {
Expand Down
12 changes: 8 additions & 4 deletions ui/artalk/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import type { ArtalkConfig, CommentData, ListFetchParams, ContextApi, EventPaylo
import type { TInjectedServices } from './service'
import Api from './api'

import * as Utils from './lib/utils'
import * as marked from './lib/marked'
import { mergeDeep } from './lib/merge-deep'
import { CheckerCaptchaPayload, CheckerPayload } from './components/checker'

import { DataManager } from './data'
Expand All @@ -27,10 +27,10 @@ class Context implements ContextApi {
/* Event Manager */
private events = new EventManager<EventPayloadMap>()

public constructor(conf: ArtalkConfig, $root?: HTMLElement) {
public constructor(conf: ArtalkConfig) {
this.conf = conf

this.$root = $root || document.createElement('div')
this.$root = conf.el as HTMLElement
this.$root.classList.add('artalk')
this.$root.innerHTML = ''

Expand Down Expand Up @@ -134,10 +134,14 @@ class Context implements ContextApi {
}

public updateConf(nConf: Partial<ArtalkConfig>): void {
this.conf = Utils.mergeDeep(this.conf, handelCustomConf(nConf))
this.conf = mergeDeep(this.conf, handelCustomConf(nConf, false))
this.events.trigger('conf-loaded', this.conf)
}

public getConf(): ArtalkConfig {
return this.conf
}

public getMarked() {
return marked.getInstance()
}
Expand Down
65 changes: 65 additions & 0 deletions ui/artalk/src/lib/merge-deep.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { describe, expect, it } from 'vitest'
import { mergeDeep } from './merge-deep'

describe('Normal operations', () => {
it('should merge objects (1 level)', () => {
expect(mergeDeep({ a: 1 }, { b: 2 })).toEqual({ a: 1, b: 2 })
expect(mergeDeep({ a: 1 }, { a: 2 })).toEqual({ a: 2 })
})

it('should merge objects (2 levels)', () => {
expect(mergeDeep({ a: { b: 1 } }, { a: { c: 2 } })).toEqual({ a: { b: 1, c: 2 } })
expect(mergeDeep({ a: { b: 1 } }, { a: { b: 2 } })).toEqual({ a: { b: 2 } })
})

it('should merge objects (3 levels)', () => {
expect(mergeDeep({ a: { b: { c: 1 } } }, { a: { b: { d: 2 } } })).toEqual({ a: { b: { c: 1, d: 2 } } })
expect(mergeDeep({ a: { b: { c: 1 } } }, { a: { b: { c: 2 } } })).toEqual({ a: { b: { c: 2 } } })
})
})

describe('Array merge', () => {
it('should merge arrays (1 level)', () => {
expect(mergeDeep({ a: [1] }, { a: [2] })).toEqual({ a: [1, 2] })
})
it('should merge arrays (2 levels)', () => {
expect(mergeDeep({ a: { b: [1] } }, { a: { b: [2] } })).toEqual({ a: { b: [1, 2] } })
})
it('should merge arrays (3 levels)', () => {
expect(mergeDeep({ a: { b: { c: [1] } } }, { a: { b: { c: [2] } } })).toEqual({ a: { b: { c: [1, 2] } } })
})
})

describe('Prevent in-place modify: mergeDeep(a, b)', () => {
it('should not modify a', () => {
const a: any = { a: 1, arr: [1, 2, 3] }
mergeDeep(a, { a: 2, arr: [4, 5, 6] })
expect(a).toEqual({ a: 1, arr: [1, 2, 3] })
})

it('should not modify b', () => {
const b: any = { a: 1, arr: [1, 2, 3] }
mergeDeep({ a: 2, arr: [4, 5, 6] }, b)
expect(b).toEqual({ a: 1, arr: [1, 2, 3] })
})
})

describe('Merge special types', () => {
const testItem = (name: string, val: any) => {
it(name, () => {
expect(mergeDeep({ a: val }, { b: val })).toEqual({ a: val, b: val })
})
}

const dom = document.createElement('div')
testItem('should can keep dom, not deep recursion', dom)

const fn = () => {}
testItem('should can keep function', fn)

const date = new Date()
testItem('should can keep date', date)

const reg = /abc/
testItem('should can keep regex', reg)
})
32 changes: 32 additions & 0 deletions ui/artalk/src/lib/merge-deep.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Performs a deep merge of objects and returns new object.
* Does not modify objects (immutable) and merges arrays via concatenation.
*
* @param objects - Objects to merge
* @returns New object with merged key/values
*/
export function mergeDeep<T>(...objects: any[]): T {
const isObject = (obj: any) => obj && typeof obj === "object" && obj.constructor === Object;

return objects.reduce((prev, obj) => {
Object.keys(obj ?? {}).forEach((key) => {
// Avoid prototype pollution
if (key === "__proto__" || key === "constructor" || key === "prototype") {
return
}

const pVal = prev[key]
const oVal = obj[key]

if (Array.isArray(pVal) && Array.isArray(oVal)) {
prev[key] = pVal.concat(...oVal)
} else if (isObject(pVal) && isObject(oVal)) {
prev[key] = mergeDeep(pVal, oVal)
} else {
prev[key] = oVal
}
})

return prev
}, {})
}
30 changes: 0 additions & 30 deletions ui/artalk/src/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,33 +204,3 @@ export function getURLBasedOnApi(opts: { base: string, path: string }) {
export function getURLBasedOn(baseURL: string, path: string) {
return `${baseURL.replace(/\/$/, '')}/${path.replace(/^\//, '')}`
}

/**
* Performs a deep merge of `source` into `target`.
* Mutates `target` only but not its objects and arrays.
*
* @author inspired by [jhildenbiddle](https://stackoverflow.com/a/48218209).
* @link https://gist.github.com/ahtcx/0cd94e62691f539160b32ecda18af3d6
*/
export function mergeDeep(target: any, source: any) {
const isObject = (obj: any) => obj && typeof obj === 'object'

if (!isObject(target) || !isObject(source)) {
return source
}

Object.keys(source).forEach(key => {
const targetValue = target[key]
const sourceValue = source[key]

if (Array.isArray(targetValue) && Array.isArray(sourceValue)) {
target[key] = targetValue.concat(sourceValue)
} else if (isObject(targetValue) && isObject(sourceValue)) {
target[key] = mergeDeep({ ...targetValue}, sourceValue)
} else {
target[key] = sourceValue
}
})

return target
}
5 changes: 5 additions & 0 deletions ui/artalk/src/plugins/conf-remoter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ import { showErrorDialog } from '../components/error-dialog'

export const ConfRemoter: ArtalkPlugin = (ctx) => {
ctx.on('inited', () => {
if (ctx.conf.immediateFetch === false) return
ctx.trigger('conf-fetch')
})

ctx.on('conf-fetch', () => {
loadConf(ctx)
})
}
Expand Down
16 changes: 16 additions & 0 deletions ui/artalk/tests/setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { vi } from 'vitest'

// @see https://github.com/vitest-dev/vitest/issues/821
Object.defineProperty(window, 'matchMedia', {
writable: true,
value: vi.fn().mockImplementation(query => ({
matches: false,
media: query,
onchange: null,
addListener: vi.fn(), // deprecated
removeListener: vi.fn(), // deprecated
addEventListener: vi.fn(),
removeEventListener: vi.fn(),
dispatchEvent: vi.fn(),
})),
})
Loading

0 comments on commit 4c2646e

Please sign in to comment.