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

refactor: optimize type and deserialize for config #837

Merged
merged 12 commits into from
Jan 3, 2024

Conversation

PeachScript
Copy link
Member

@PeachScript PeachScript commented Jan 2, 2024

对配置文件的类型及反序列化值做优化,改动点:

  1. manifest: boolmanifest_config: { ... } 合并为 manifest: Option<{ ... }>
  2. hmr: boolhmr_config: { ... } 合并为 hmr: Option<{ ... }>
  3. px2rem: boolpx2rem_onfig: { ... } 合并为 px2rem: Option<{ ... }>
  4. umd: String 调整为 umd: Option<String>,用 None 替代特殊字符串值 none
  5. CodeSplittingStrategy::None 调整为 Option<CodeSplittingStrategy>,用 None 替代特殊枚举值 None
  6. treeShake 调整为 _treeShaking,标记为私有配置并从文档中去除
  7. TreeShaking::None 调整为 Option<CodeSplittingStrategy>,用 None 替代特殊枚举值 None
  8. DevtoolConfig::None 调整为 Option<DevtoolConfig>,用 None 替代特殊枚举值 None
  9. 去掉无用的 less 配置项
  10. 所有 Option<T> 均支持 false 值并禁用 null,前者更符合 JavaScript 用户习惯,Mako 会自动将 false 反序列化为 None 以便后续逻辑消费 Option
  11. 更新 bundler-okam 的配置转换逻辑以符合新的类型

学到的经验(踩到的坑),config crate 里 add_source 覆盖的前提是类型相同做合并,比如(伪代码):

// hmr 永远是 false
config.builder()
  .add_source("{ hmr: false }")
  .add_source("{ hmr: {} }")

// hmr 可以按预期合并
config.builder()
  .add_source("{ hmr: {} }")
  .add_source("{ hmr: { port: 123 } }")

所以如果希望提供默认关闭的配置项,不应该在第一个 add_source 的内容里将它设置为 false,而是应该用 default attribute,比如(伪代码):

// Option 的默认值是 None,符合默认禁用的预期
struct {
  #[serde(default)]
  hmr: Option<HmrConfig>,
}

@sorrycc sorrycc merged commit 9bb02a6 into master Jan 3, 2024
8 checks passed
@delete-merged-branch delete-merged-branch bot deleted the feature/config-combine branch January 3, 2024 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants