Add dispatchEvent to fragment instances #32813
Conversation
|
Comparing: e5a8de8...30f04c2 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
| console.error( | ||
| 'You are attempting to dispatch an event on a disconnected ' + | ||
| 'FragmentInstance. No event was dispatched.', | ||
| ); |
There was a problem hiding this comment.
I wonder if this is necessary. It might backfire if you don't know, now you have to check with something like getRootNode() instead.
There was a problem hiding this comment.
Yeah probably more annoying than anything. Updated for it to just exit silently.
| } | ||
| return true; | ||
| } | ||
| return parentHostInstance.dispatchEvent(event); |
There was a problem hiding this comment.
It's interesting because we have addEventListener on the ref. Shouldn't those listeners be invoked too? It's kind of weird that your own listeners wouldn't fire.
We could manually invoke them but unfortunately the timing wouldn't be quite right in the capture/bubble phase.
One simple thing we can do is:
| return parentHostInstance.dispatchEvent(event); | |
| if (this.__eventListeners !== null || !event.bubbles) { | |
| const temp = document.createTextNode('') | |
| // addEventListener for all the ones in __eventListeners | |
| parentHostInstance.appendChild(temp); | |
| return temp.dispatchEvent(event); | |
| parentHostInstance.removeChild(temp); | |
| } else { | |
| return parentHostInstance.dispatchEvent(event); | |
| } |
Most of the times the simple path is hit because you're unlike to both listen and dispatch to the same event type and even more unlikely to have non-bubbling dispatches.
There was a problem hiding this comment.
Also felt like it was a bit awkward but wasn't sure about the right semantics. This feels reasonable to me, updated to include this logic.
| @@ -2331,6 +2332,23 @@ function removeEventListenerFromChild( | |||
| return false; | |||
| } | |||
| // $FlowFixMe[prop-missing] | |||
| FragmentInstance.prototype.dispatchEvent = function ( | |||
| this: FragmentInstanceType, | |||
| event: Event, | |||
There was a problem hiding this comment.
We should check if this event.bubbles is true because if it's false, it should only fire on the fragment's own listeners and no the parent node. This can be solved by the same technique as self listeners.
cc19468 to
19a18a6
Compare
19a18a6 to
fc9cdab
Compare
There was a problem hiding this comment.
@jackpope and I discussed this together. The way I interpreted this diff is as "event propagation for fragment refs" and we discussed making sure that if one of the fragment ref handles calls event.stopPropagation that this would work under this implementation
`fragmentInstance.dispatchEvent(evt)` calls `element.dispatchEvent(evt)` on the fragment's host parent. This mimics bubbling if the `fragmentInstance` could receive an event itself. If the parent is disconnected, there is a dev warning and no event is dispatched. DiffTrain build for [8a8df5d](8a8df5d)
`fragmentInstance.dispatchEvent(evt)` calls `element.dispatchEvent(evt)` on the fragment's host parent. This mimics bubbling if the `fragmentInstance` could receive an event itself. If the parent is disconnected, there is a dev warning and no event is dispatched. DiffTrain build for [8a8df5d](8a8df5d)
`fragmentInstance.dispatchEvent(evt)` calls `element.dispatchEvent(evt)` on the fragment's host parent. This mimics bubbling if the `fragmentInstance` could receive an event itself. If the parent is disconnected, there is a dev warning and no event is dispatched. DiffTrain build for [8a8df5d](facebook@8a8df5d)
`fragmentInstance.dispatchEvent(evt)` calls `element.dispatchEvent(evt)` on the fragment's host parent. This mimics bubbling if the `fragmentInstance` could receive an event itself. If the parent is disconnected, there is a dev warning and no event is dispatched. DiffTrain build for [8a8df5d](facebook@8a8df5d)
fragmentInstance.dispatchEvent(evt)callselement.dispatchEvent(evt)on the fragment's host parent. This mimics bubbling if thefragmentInstancecould receive an event itself.If the parent is disconnected, there is a dev warning and no event is dispatched.