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

fix: config always get override by defaults #720

Closed
wants to merge 1 commit into from

Conversation

frostming
Copy link
Contributor

@frostming frostming commented Dec 25, 2023

Signed-off-by: Frost Ming me@frostming.com

Because confRemoter calls updateConf, which in turn calls handleCustomConf and which calls mergeDeep(Defaults, userConf)

The values from the default will always take precedence to override the existing values, this is unexpected.

updateConf
  ↑ mergeDeep(this.conf, handleCustomConf(nConf))
                                ↑ mergeDeep(Defaults, nConf)

This patch fixes #718

Signed-off-by: Frost Ming <me@frostming.com>
@frostming
Copy link
Contributor Author

Oh, I didn't notice #719, feel free to choose either one.

@qwqcode
Copy link
Member

qwqcode commented Dec 25, 2023

Thank you for your time to careful review and PR. Based on the solution offered in the PR, I have refactored the related API configurations (opened a new PR #722 and included unit tests), which should address this issue. The unit tests are in place to ensure that this problem won't occur again in the future. 😃

@qwqcode qwqcode closed this in #722 Dec 26, 2023
@frostming frostming deleted the fix/config-override branch December 28, 2023 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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