MultiHook should support different types of hooks#199
MultiHook should support different types of hooks#199jianyi-gronk wants to merge 1 commit intowebpack:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #199 +/- ##
=======================================
Coverage 97.97% 97.97%
=======================================
Files 13 13
Lines 692 692
Branches 113 118 +5
=======================================
Hits 678 678
Misses 13 13
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
|
Got it, thanks for the explanation. Just to confirm: MultiHook was not design to use with SyncHook and AsyncHook, so, it's not expected to support adding the same interceptor to both a SyncHook and an AsyncHook at the same time via MultiHook, right? |
|
Yes, it was design only to use the same class, i.e. you can use |
In the current implementation of MultiHook, there is no check for whether a method exists on each hook before invoking it, I think this maybe lead to issues.
For example:
I think a better approach would be to skip the call on any hook that does not support the method, rather than throwing an error.