fix: lifecycle hook should trigger only one time #168
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
whxaxes
requested review from
atian25,
noahziheng,
Beace,
JerrysShan,
hyj1991 and
DuanPengfei
Codecov Report
@@ Coverage Diff @@
## master #168 +/- ##
=======================================
Coverage 89.59% 89.60%
=======================================
Files 51 51
Lines 1096 1097 +1
Branches 181 181
=======================================
+ Hits 982 983 +1
Misses 114 114
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
改动思路或许有点过重了？ 而且按照 Spec，生命周期中的 emitHook 其实应该只允许执行一次且必须按照 insert 的顺序执行； 是因为历史问题而没有加校验，是不是应该在 LifecycleManager 里面补全校验逻辑，既能解决问题，也无需引入新概念，反正都是 BREAKING CHANGE |
|
@noahziheng 阔以，我去改一下 LifecycleManager ，state 这个我也不确定是不是合适，先提出来给大家 review |
whxaxes
force-pushed
the
feat-add-state-control
branch
from
789ef01 to
8acdeb9
Compare
whxaxes
changed the title
|
@noahziheng 妥了 |
noahziheng
approved these changes
Aug 11, 2022
whxaxes
changed the title
hyj1991
reviewed
Aug 11, 2022
| @@ -68,6 +68,8 @@ export class LifecycleManager { | |||
| if (!Array.isArray(fnList)) { | |||
| return; | |||
| } | |||
| // lifecycle hook should only trigger one time | |||
| this.hookFnMap.delete(hookName); | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
设计上确实应该执行一次，如果有多次触发的类似事件分发机制的场景，就不该通过 Lifecycle 做，应该考虑 Trigger 或者 EventEmitter
noahziheng
force-pushed
the
feat-add-state-control
branch
from
8acdeb9 to
a670349
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
npm testpassesDescription of change
生命周期的 hooks 应该只执行一次。