Skip to content
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

In version 2.7.x, mergeDeep causes some configurations to be overridden by the default configuration. #718

Closed
syfxlin opened this issue Dec 24, 2023 · 1 comment · Fixed by #722
Labels
bug Something isn't working

Comments

@syfxlin
Copy link
Member

syfxlin commented Dec 24, 2023

Description

最近我升级了博客中 Artalk 版本到 2.7.2,出现了死循环的问题,从而导致 Artalk 无法正常显示 (Issue 1),如下:

image

但是这个问题已经在 #717 中修复,我尝试修改 node_modules 写入该补丁。然后我发现无法正常显示的问题被修复了,却出现了另一个问题,Artalk 请求的 API 变成了当前的 domain (Issue 2)。

image

Reproduction steps

Issue 1

Issue 2

  • 在 Issue 1 的基础上,修改 node_modules/artalk/dist/Artalk.es.js#4336 行,将 const conf = mergeDeep(defaults, customConf); 改成 const conf = mergeDeep({ ...defaults }, customConf);
  • 运行 npm run dev

Tracks

Issue 1

死循环会导致调用 Context.updateConf 失败,而 Artalk 在 conf-remoter.ts#L26 调用了该方法,失败后转到 conf-remoter.ts#28,此时会再次失败进入 conf-remoter.ts#L53,此后不再更新 UI,导致显示空白。

Issue 2

在 Artalk 初始化过程中会多次调用 Context.updateConf,当首次进入时 this.conf 是自定义的配置,而 nConf 仅包含需要更新的配置,如下(请关注 serversite 字段):

image

image

之后进入了 handelCustomConf 方法,由于 nConf 并不包含 serversite 字段,因此在 mergeDeep 之后会被默认的配置覆盖,如下:

image

在出了 handelCustomConf 方法后,mergeDeep 会将 this.conf 中的 serversite 字段设置成默认配置,如下:

image

Versions

  • Artalk UI:2.7.2
  • Artalk API:2.7.2

Possible solutions

合并默认配置的流程在 new Artalk(config) 处理一次即可,在处理一次之后 Artalk 中维护的配置已经是完整的,因此后续更新的时候可以去掉合并默认配置的流程。

另外 #717 的修复并不完整,由于默认配置中包含了嵌套的对象,展开语法是浅拷贝,因此默认值中的嵌套对象可能被新的配置覆盖。

@qwqcode
Copy link
Member

qwqcode commented Dec 25, 2023

感谢仔细检查!!(´இ皿இ`)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants