Calling "DISPOSE" action doesn't result in removing navigation listeners

[catamphetamine]

In createNavigationListenerMiddleware.js it handles a ActionTypes.DISPOSE action:

farce/src/createNavigationListenerMiddleware.js

Lines 223 to 228 in 5f79cb9

	case ActionTypes.DISPOSE:
	if (listenerEntries.length > 0 && onBeforeUnload) {
	window.removeEventListener('beforeunload', onBeforeUnload);
	}
	return next(action);

But there seems to be a couple of issues with that code:

• The condition listenerEntries.length > 0 && onBeforeUnload seems to be incorrect:
  • onBeforeUnload is 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 listenerEntries themselves 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