diff --git a/src/createTransitionHookMiddleware.js b/src/createTransitionHookMiddleware.js index 7be7450..89a3c0a 100644 --- a/src/createTransitionHookMiddleware.js +++ b/src/createTransitionHookMiddleware.js @@ -69,16 +69,46 @@ export default function createTransitionHookMiddleware({ let nextStep = null; let hooks = []; + const onBeforeUnload = useBeforeUnload + ? /* istanbul ignore next: not testable with Karma */ + (event) => { + const syncResult = runHooks(hooks, null, (result) => result); + + if (syncResult === true || syncResult === undefined) { + // An asynchronous transition hook usually means there will be a + // custom confirm dialog. However, we'll already be showing the + // before unload dialog, and there's no way to prevent the custom + // dialog from showing. In such cases, the application code will + // need to explicitly handle the null location anyway, so don't + // potentially show two confirmation dialogs. + return undefined; + } + + const resultSafe = syncResult || ''; + + event.returnValue = resultSafe; // eslint-disable-line no-param-reassign + return resultSafe; + } + : null; + function addHook(hook) { + // Add the beforeunload event listener only as needed, as its presence + // prevents the page from being added to the page navigation cache. + if (hooks.length === 0 && onBeforeUnload) { + window.addEventListener('beforeunload', onBeforeUnload); + } + hooks.push(hook); return () => { hooks = hooks.filter((item) => item !== hook); + + if (hooks.length === 0 && onBeforeUnload) { + window.removeEventListener('beforeunload', onBeforeUnload); + } }; } - let onBeforeUnload = null; - function transitionHookMiddleware({ dispatch }) { return (next) => (action) => { const { type, payload } = action; @@ -90,33 +120,6 @@ export default function createTransitionHookMiddleware({ } switch (type) { - case ActionTypes.INIT: - // Only attach this listener once. - if (useBeforeUnload && !onBeforeUnload) { - /* istanbul ignore next: not testable with Karma */ - onBeforeUnload = (event) => { - const syncResult = runHooks(hooks, null, (result) => result); - - if (syncResult === true || syncResult === undefined) { - // An asynchronous transition hook usually means there will be - // a custom confirm dialog. However, we'll already be showing - // the before unload dialog, and there's no way to prevent the - // custom dialog from showing. In such cases, the application - // code will need to explicitly handle the null location - // anyway, so don't potentially show two confirmation dialogs. - return undefined; - } - - const resultSafe = syncResult || ''; - - event.returnValue = resultSafe; // eslint-disable-line no-param-reassign - return resultSafe; - }; - - window.addEventListener('beforeunload', onBeforeUnload); - } - - return next(action); case ActionTypes.TRANSITION: return runAllowTransition(hooks, payload, (allowTransition) => { if (!allowTransition) { @@ -204,9 +207,8 @@ export default function createTransitionHookMiddleware({ return undefined; } case ActionTypes.DISPOSE: - if (onBeforeUnload) { + if (hooks.length > 0 && onBeforeUnload) { window.removeEventListener('beforeunload', onBeforeUnload); - onBeforeUnload = null; } return next(action); diff --git a/test/createTransitionHookMiddleware.test.js b/test/createTransitionHookMiddleware.test.js index 2daaec7..a3a9884 100644 --- a/test/createTransitionHookMiddleware.test.js +++ b/test/createTransitionHookMiddleware.test.js @@ -417,28 +417,44 @@ describe('createTransitionHookMiddleware', () => { }); }); - it('should manage event listeners with useBeforeUnload', () => { - // Get rid of the old store. We'll replace it with a new one. - store.dispatch(Actions.dispose()); + describe('useBeforeUnload', () => { + beforeEach(() => { + // Get rid of the old store. We'll replace it with a new one. + store.dispatch(Actions.dispose()); - sandbox.stub(window, 'addEventListener'); - sandbox.stub(window, 'removeEventListener'); + sandbox.stub(window, 'addEventListener'); + sandbox.stub(window, 'removeEventListener'); - store = createStore( - () => null, - createHistoryEnhancer({ protocol, useBeforeUnload: true }), - ); + store = createStore( + () => null, + createHistoryEnhancer({ protocol, useBeforeUnload: true }), + ); - expect(window.addEventListener).not.to.have.been.called(); - store.dispatch(Actions.init()); - expect(window.addEventListener) - .to.have.been.calledOnce() - .and.to.have.been.called.with('beforeunload'); + store.dispatch(Actions.init()); + }); - expect(window.removeEventListener).not.to.have.been.called(); - store.dispatch(Actions.dispose()); - expect(window.removeEventListener) - .to.have.been.calledOnce() - .and.to.have.been.called.with('beforeunload'); + it('should manage event listener on adding and removing hook', () => { + expect(window.addEventListener).not.to.have.been.called(); + const removeHook = store.farce.addTransitionHook(() => null); + expect(window.addEventListener) + .to.have.been.calledOnce() + .and.to.have.been.called.with('beforeunload'); + + expect(window.removeEventListener).not.to.have.been.called(); + removeHook(); + expect(window.removeEventListener) + .to.have.been.calledOnce() + .and.to.have.been.called.with('beforeunload'); + }); + + it('should remove event listener on dispose', () => { + store.farce.addTransitionHook(() => null); + + expect(window.removeEventListener).not.to.have.been.called(); + store.dispatch(Actions.dispose()); + expect(window.removeEventListener) + .to.have.been.calledOnce() + .and.to.have.been.called.with('beforeunload'); + }); }); });