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: should call config hook in idle callback #218

Merged
merged 8 commits into from
Dec 5, 2022

Conversation

whxaxes
Copy link
Member

@whxaxes whxaxes commented Dec 5, 2022

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

config 的 lifecycle hook 是 loader 事件触发,当不存在 config 文件的时候不会触发 loader 的 before/after 事件,导致 config 的 lifecycle hook 也不会触发

所以在 loader 事件中加个 idle 事件,用于当某个 loader 没有被触发时回调,从而解决 config loader 没有使用时也能触发 configWillLoad 及 configDidLoad

根据 DEFAULT_LOADER_LIST_WITH_ORDER 进行分组,分组后按顺序执行,保证内置的每个 loader 的 before/after 都会触发

src/application.ts Outdated Show resolved Hide resolved
src/loader/factory.ts Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2022

Codecov Report

Base: 92.42% // Head: 92.42% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (894aeb8) compared to base (9f3b77c).
Patch coverage: 93.33% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #218   +/-   ##
=======================================
  Coverage   92.42%   92.42%           
=======================================
  Files          55       55           
  Lines        1293     1294    +1     
  Branches      218      216    -2     
=======================================
+ Hits         1195     1196    +1     
  Misses         98       98           
Impacted Files Coverage Δ
src/loader/factory.ts 92.78% <93.33%> (+0.07%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

src/scanner/scan.ts Outdated Show resolved Hide resolved
@whxaxes
Copy link
Member Author

whxaxes commented Dec 5, 2022

CI 挂了,我看一下

Copy link
Member

@hyj1991 hyj1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

Copy link
Member

@noahziheng noahziheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@noahziheng noahziheng merged commit 3b4deac into master Dec 5, 2022
@whxaxes whxaxes deleted the fix-should-call-confighook-in-idle branch December 6, 2022 02:01
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.

6 participants