farce
farce copied to clipboard
Calling "DISPOSE" action doesn't result in removing navigation listeners
In createNavigationListenerMiddleware.js it handles a ActionTypes.DISPOSE action:
https://github.com/4Catalyzer/farce/blob/5f79cb9c2bbae44a8c4ab7e9ae0afa8b5657f966/src/createNavigationListenerMiddleware.js#L223-L228
But there seems to be a couple of issues with that code:
- The condition
listenerEntries.length > 0 && onBeforeUnloadseems to be incorrect:onBeforeUnloadis a function name rather than a variable name, so it always exists.- A more appropriate condition would've been
listenerEntries.some((item) => item.beforeUnload)
- Safe coding practices would also require emptying the list of
listenerEntriesthemselves during the "dispose" phase.
The code above also just happens to be written in such a way that it both isn't correct but at the same time always has the correct outcome because it always removes that event listener on DISPOSE which it's supposed to do. So it's a bit funny that it introduces no bugs at all.
My fix would've been:
case ActionTypes.DISPOSE:
if (listenerEntries.some((item) => item.beforeUnload)) {
removeBeforeUnloadListener(onBeforeUnload);
}
listenerEntries = [];
return next(action);
If anyone's interested, I published this workaround as part of navigation-stack package:
https://www.npmjs.com/package/navigation-stack