Skip to content

Commit

Permalink
Remove FlashMessagesProvider in favor of mounting logic directly w/ p…
Browse files Browse the repository at this point in the history
…rops

- send history as prop
- refactor out now-unnecessary listenToHistory (we can just do it directly in afterMount without worrying about duplicate react rerenders)
- add mount helper

Tests:
- refactor history.listen mock to mockHistory (so that set_message_helpers can use it as well)
- use mountFlashMessagesLogic + create an even shorter mount() helper (credit to @JasonStoltz for the idea!)
- refactor out DEFAULT_VALUES since we're not really using it anywhere else in the file, and it's not super applicable to this store
- update history listener tests to account for logic occurring immediately on mount
  • Loading branch information
cee-chen committed Sep 22, 2020
1 parent b763147 commit 2d13707
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 113 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export const mockHistory = {
location: {
pathname: '/current-path',
},
listen: jest.fn(() => jest.fn()),
};
export const mockLocation = {
key: 'someKey',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import { I18nProvider } from '@kbn/i18n/react';
import { AppMountParameters, CoreStart, ApplicationStart, ChromeBreadcrumb } from 'src/core/public';
import { ClientConfigType, ClientData, PluginsSetup } from '../plugin';
import { LicenseProvider } from './shared/licensing';
import { FlashMessagesProvider } from './shared/flash_messages';
import { mountHttpLogic } from './shared/http';
import { mountFlashMessagesLogic } from './shared/flash_messages';
import { IExternalUrl } from './shared/enterprise_search_url';
import { IInitialAppData } from '../../common/types';

Expand Down Expand Up @@ -54,6 +54,8 @@ export const renderApp = (
readOnlyMode: initialData.readOnlyMode,
});

const unmountFlashMessagesLogic = mountFlashMessagesLogic({ history: params.history });

ReactDOM.render(
<I18nProvider>
<KibanaContext.Provider
Expand All @@ -67,7 +69,6 @@ export const renderApp = (
>
<LicenseProvider license$={plugins.licensing.license$}>
<Provider store={store}>
<FlashMessagesProvider history={params.history} />
<Router history={params.history}>
<App {...initialData} />
</Router>
Expand All @@ -80,6 +81,7 @@ export const renderApp = (
return () => {
ReactDOM.unmountComponentAtNode(params.element);
unmountHttpLogic();
unmountFlashMessagesLogic();
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,25 @@

import { resetContext } from 'kea';

import { FlashMessagesLogic, IFlashMessage } from './flash_messages_logic';
import { mockHistory } from '../../__mocks__';

import { FlashMessagesLogic, mountFlashMessagesLogic, IFlashMessage } from './';

describe('FlashMessagesLogic', () => {
const DEFAULT_VALUES = {
messages: [],
queuedMessages: [],
historyListener: null,
};
const mount = () => mountFlashMessagesLogic({ history: mockHistory as any });

beforeEach(() => {
jest.clearAllMocks();
resetContext({});
});

it('has expected default values', () => {
FlashMessagesLogic.mount();
expect(FlashMessagesLogic.values).toEqual(DEFAULT_VALUES);
it('has default values', () => {
mount();
expect(FlashMessagesLogic.values).toEqual({
messages: [],
queuedMessages: [],
historyListener: expect.any(Function),
});
});

describe('setFlashMessages()', () => {
Expand All @@ -33,7 +35,7 @@ describe('FlashMessagesLogic', () => {
{ type: 'info', message: 'Everything is fine, nothing is ruined' },
];

FlashMessagesLogic.mount();
mount();
FlashMessagesLogic.actions.setFlashMessages(messages);

expect(FlashMessagesLogic.values.messages).toEqual(messages);
Expand All @@ -42,7 +44,7 @@ describe('FlashMessagesLogic', () => {
it('automatically converts to an array if a single message obj is passed in', () => {
const message = { type: 'success', message: 'I turn into an array!' } as IFlashMessage;

FlashMessagesLogic.mount();
mount();
FlashMessagesLogic.actions.setFlashMessages(message);

expect(FlashMessagesLogic.values.messages).toEqual([message]);
Expand All @@ -51,7 +53,7 @@ describe('FlashMessagesLogic', () => {

describe('clearFlashMessages()', () => {
it('sets messages back to an empty array', () => {
FlashMessagesLogic.mount();
mount();
FlashMessagesLogic.actions.setFlashMessages('test' as any);
FlashMessagesLogic.actions.clearFlashMessages();

Expand All @@ -63,7 +65,7 @@ describe('FlashMessagesLogic', () => {
it('sets an array of messages', () => {
const queuedMessage: IFlashMessage = { type: 'error', message: 'You deleted a thing' };

FlashMessagesLogic.mount();
mount();
FlashMessagesLogic.actions.setQueuedMessages(queuedMessage);

expect(FlashMessagesLogic.values.queuedMessages).toEqual([queuedMessage]);
Expand All @@ -72,7 +74,7 @@ describe('FlashMessagesLogic', () => {

describe('clearQueuedMessages()', () => {
it('sets queued messages back to an empty array', () => {
FlashMessagesLogic.mount();
mount();
FlashMessagesLogic.actions.setQueuedMessages('test' as any);
FlashMessagesLogic.actions.clearQueuedMessages();

Expand All @@ -83,30 +85,25 @@ describe('FlashMessagesLogic', () => {
describe('history listener logic', () => {
describe('setHistoryListener()', () => {
it('sets the historyListener value', () => {
FlashMessagesLogic.mount();
mount();
FlashMessagesLogic.actions.setHistoryListener('test' as any);

expect(FlashMessagesLogic.values.historyListener).toEqual('test');
});
});

describe('listenToHistory()', () => {
describe('on mount', () => {
it('listens for history changes and clears messages on change', () => {
FlashMessagesLogic.mount();
mount();
expect(mockHistory.listen).toHaveBeenCalled();

FlashMessagesLogic.actions.setQueuedMessages(['queuedMessages'] as any);
jest.spyOn(FlashMessagesLogic.actions, 'clearFlashMessages');
jest.spyOn(FlashMessagesLogic.actions, 'setFlashMessages');
jest.spyOn(FlashMessagesLogic.actions, 'clearQueuedMessages');
jest.spyOn(FlashMessagesLogic.actions, 'setHistoryListener');

const mockListener = jest.fn(() => jest.fn());
const history = { listen: mockListener } as any;
FlashMessagesLogic.actions.listenToHistory(history);

expect(mockListener).toHaveBeenCalled();
expect(FlashMessagesLogic.actions.setHistoryListener).toHaveBeenCalled();

const mockHistoryChange = (mockListener.mock.calls[0] as any)[0];
const mockHistoryChange = (mockHistory.listen.mock.calls[0] as any)[0];
mockHistoryChange();
expect(FlashMessagesLogic.actions.clearFlashMessages).toHaveBeenCalled();
expect(FlashMessagesLogic.actions.setFlashMessages).toHaveBeenCalledWith([
Expand All @@ -116,19 +113,20 @@ describe('FlashMessagesLogic', () => {
});
});

describe('beforeUnmount', () => {
it('removes history listener on unmount', () => {
describe('on unmount', () => {
it('removes history listener', () => {
const mockUnlistener = jest.fn();
const unmount = FlashMessagesLogic.mount();
mockHistory.listen.mockReturnValueOnce(mockUnlistener);

FlashMessagesLogic.actions.setHistoryListener(mockUnlistener);
const unmount = mount();
unmount();

expect(mockUnlistener).toHaveBeenCalled();
});

it('does not crash if no listener exists', () => {
const unmount = FlashMessagesLogic.mount();
const unmount = mount();
FlashMessagesLogic.actions.setHistoryListener(null as any);
unmount();
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export interface IFlashMessagesActions {
clearFlashMessages(): void;
setQueuedMessages(messages: IFlashMessage | IFlashMessage[]): { messages: IFlashMessage[] };
clearQueuedMessages(): void;
listenToHistory(history: History): History;
setHistoryListener(historyListener: Function): { historyListener: Function };
}

Expand All @@ -38,7 +37,6 @@ export const FlashMessagesLogic = kea<MakeLogicType<IFlashMessagesValues, IFlash
clearFlashMessages: () => null,
setQueuedMessages: (messages) => ({ messages: convertToArray(messages) }),
clearQueuedMessages: () => null,
listenToHistory: (history) => history,
setHistoryListener: (historyListener) => ({ historyListener }),
},
reducers: {
Expand All @@ -63,21 +61,31 @@ export const FlashMessagesLogic = kea<MakeLogicType<IFlashMessagesValues, IFlash
},
],
},
listeners: ({ values, actions }) => ({
listenToHistory: (history) => {
events: ({ props, values, actions }) => ({
afterMount: () => {
// On React Router navigation, clear previous flash messages and load any queued messages
const unlisten = history.listen(() => {
const unlisten = props.history.listen(() => {
actions.clearFlashMessages();
actions.setFlashMessages(values.queuedMessages);
actions.clearQueuedMessages();
});
actions.setHistoryListener(unlisten);
},
}),
events: ({ values }) => ({
beforeUnmount: () => {
const { historyListener: removeHistoryListener } = values;
if (removeHistoryListener) removeHistoryListener();
},
}),
});

/**
* Mount/props helper
*/
interface IFlashMessagesLogicProps {
history: History;
}
export const mountFlashMessagesLogic = (props: IFlashMessagesLogicProps) => {
FlashMessagesLogic(props);
const unmount = FlashMessagesLogic.mount();
return unmount;
};

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export {
IFlashMessage,
IFlashMessagesValues,
IFlashMessagesActions,
mountFlashMessagesLogic,
} from './flash_messages_logic';
export { FlashMessagesProvider } from './flash_messages_provider';
export { flashAPIErrors } from './handle_api_errors';
export { setSuccessMessage, setErrorMessage, setQueuedSuccessMessage } from './set_message_helpers';
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { mockHistory } from '../../__mocks__';

import {
FlashMessagesLogic,
mountFlashMessagesLogic,
setSuccessMessage,
setErrorMessage,
setQueuedSuccessMessage,
Expand All @@ -15,7 +18,7 @@ describe('Flash Message Helpers', () => {
const message = 'I am a message';

beforeEach(() => {
FlashMessagesLogic.mount();
mountFlashMessagesLogic({ history: mockHistory as any });
});

it('setSuccessMessage()', () => {
Expand Down

0 comments on commit 2d13707

Please sign in to comment.