refactor(eventStack): make eventStack immutable #2837
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2837 +/- ##
=======================================
Coverage 99.66% 99.66%
=======================================
Files 161 161
Lines 2722 2722
=======================================
Hits 2713 2713
Misses 9 9Continue to review full report at Codecov.
|
| return | ||
| } | ||
|
|
||
| const recentHandler = [...this.handlers].pop() |
There was a problem hiding this comment.
EDIT - Oh, this is a Set, not an array... you can ignore my original
Rather than cloning the entire array to get the last handler, you can directly access the last item:
const recentHandler = this.handlers.slice(-1)[0]
// or
const recentHandler = this.handlers[this.handlers.length - 1]Note, the spread approach, the slice approach, and the length approach all avoid mutating the this.handlers array, however, the recentHandler is not a unique instance. It is the same instance as was originally provided to the array of handlers. I'm not sure if this matters to you or not.
| import normalizeHandlers from './normalizeHandlers' | ||
| import normalizeTarget from './normalizeTarget' | ||
|
|
||
| export default class EventStack { |
There was a problem hiding this comment.
It would be helpful to have a description above the EventSet, EventStack, and EventPool. It is not immediately clear how the 3 operate together.
There was a problem hiding this comment.
@levithomason I've added a README file with description
…React into refactor/event-stack Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
|
Merging for progress. Any doc tweaks can happen in another PR 💪 |
|
Released in |
Why?
Sometime ago I tried to attack #1473, I've fully reworked the
Sidebarcomponent and docs. But then I found a strange problem when tested the component: event handlers simply called before they should be called. I've found the problem, it was in the mutable array ofhandlersin theEventTarget.This PR performs the deep refactoring and adds more test coverage for internals of the
eventStack.The easiest way to show problem: just clone
feat/sidebar-closable-propertybranch and try to run theSidebarexamples without & with these changes.