Do not unmount apps when navigating within a single app

elastic · Dec 30, 2019 · 731a8a2 · 731a8a2
1 parent b8046c7
commit 731a8a2
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 33 deletions.
diff --git a/src/core/public/application/integration_tests/router.test.tsx b/src/core/public/application/integration_tests/router.test.tsx
@@ -29,6 +29,9 @@ describe('AppContainer', () => {
   let history: History;
   let navigate: ReturnType<typeof createRenderer>;
 
+  const mockMountersToMounters = () =>
+    new Map([...mounters].map(([appId, { mounter }]) => [appId, mounter]));
+
   beforeEach(() => {
     mounters = new Map([
       createAppMounter('app1', '<span>App 1</span>'),
@@ -38,26 +41,29 @@ describe('AppContainer', () => {
       createAppMounter('app3', '<div>App 3</div>', '/custom/path'),
     ] as Array<MockedMounterTuple<EitherApp>>);
     history = createMemoryHistory();
-    navigate = createRenderer(<AppRouter history={history} mounters={mounters} />, history.push);
+    navigate = createRenderer(
+      <AppRouter history={history} mounters={mockMountersToMounters()} />,
+      history.push
+    );
   });
 
   it('calls mount handler and returned unmount function when navigating between apps', async () => {
     const dom1 = await navigate('/app/app1');
     const app1 = mounters.get('app1')!;
 
-    expect(app1.mount).toHaveBeenCalled();
+    expect(app1.mounter.mount).toHaveBeenCalled();
     expect(dom1?.html()).toMatchInlineSnapshot(`
       "<div><div>
       basename: /app/app1
       html: <span>App 1</span>
       </div></div>"
     `);
 
-    const app1Unmount = await app1.mount.mock.results[0].value;
+    const app1Unmount = await app1.mounter.mount.mock.results[0].value;
     const dom2 = await navigate('/app/app2');
 
     expect(app1Unmount).toHaveBeenCalled();
-    expect(mounters.get('app2')!.mount).toHaveBeenCalled();
+    expect(mounters.get('app2')!.mounter.mount).toHaveBeenCalled();
     expect(dom2?.html()).toMatchInlineSnapshot(`
       "<div><div>
       basename: /app/app2
@@ -70,29 +76,52 @@ describe('AppContainer', () => {
     mounters.set(...createAppMounter('spaces', '<div>Custom Space</div>', '/spaces/fake-login'));
     mounters.set(...createAppMounter('login', '<div>Login Page</div>', '/fake-login'));
     history = createMemoryHistory();
-    navigate = createRenderer(<AppRouter history={history} mounters={mounters} />, history.push);
+    navigate = createRenderer(
+      <AppRouter history={history} mounters={mockMountersToMounters()} />,
+      history.push
+    );
 
     await navigate('/fake-login');
 
-    expect(mounters.get('spaces')!.mount).not.toHaveBeenCalled();
-    expect(mounters.get('login')!.mount).toHaveBeenCalled();
+    expect(mounters.get('spaces')!.mounter.mount).not.toHaveBeenCalled();
+    expect(mounters.get('login')!.mounter.mount).toHaveBeenCalled();
   });
 
   it('should not mount when partial route path has higher specificity', async () => {
     mounters.set(...createAppMounter('login', '<div>Login Page</div>', '/fake-login'));
     mounters.set(...createAppMounter('spaces', '<div>Custom Space</div>', '/spaces/fake-login'));
     history = createMemoryHistory();
-    navigate = createRenderer(<AppRouter history={history} mounters={mounters} />, history.push);
+    navigate = createRenderer(
+      <AppRouter history={history} mounters={mockMountersToMounters()} />,
+      history.push
+    );
 
     await navigate('/spaces/fake-login');
 
-    expect(mounters.get('spaces')!.mount).toHaveBeenCalled();
-    expect(mounters.get('login')!.mount).not.toHaveBeenCalled();
+    expect(mounters.get('spaces')!.mounter.mount).toHaveBeenCalled();
+    expect(mounters.get('login')!.mounter.mount).not.toHaveBeenCalled();
+  });
+
+  it('should not unmount when changing between apps', async () => {
+    history = createMemoryHistory();
+    navigate = createRenderer(
+      <AppRouter history={history} mounters={mockMountersToMounters()} />,
+      history.push
+    );
+
+    const { mounter, unmount } = mounters.get('app1')!;
+    await navigate('/app/app1/page1');
+    expect(mounter.mount).toHaveBeenCalledTimes(1);
+    await navigate('/app/app1/page2');
+    expect(mounter.mount).toHaveBeenCalledTimes(1);
+    expect(unmount).not.toHaveBeenCalled();
+    await navigate('/app/app2/page1');
+    expect(unmount).toHaveBeenCalledTimes(1);
   });
 
   it('calls legacy mount handler', async () => {
     await navigate('/app/legacyApp1');
-    expect(mounters.get('legacyApp1')!.mount.mock.calls[0]).toMatchInlineSnapshot(`
+    expect(mounters.get('legacyApp1')!.mounter.mount.mock.calls[0]).toMatchInlineSnapshot(`
       Array [
         Object {
           "appBasePath": "/app/legacyApp1",
@@ -104,7 +133,7 @@ describe('AppContainer', () => {
 
   it('handles legacy apps with subapps', async () => {
     await navigate('/app/baseApp');
-    expect(mounters.get('baseApp:legacyApp2')!.mount.mock.calls[0]).toMatchInlineSnapshot(`
+    expect(mounters.get('baseApp:legacyApp2')!.mounter.mount.mock.calls[0]).toMatchInlineSnapshot(`
       Array [
         Object {
           "appBasePath": "/app/baseApp",

diff --git a/src/core/public/application/integration_tests/utils.tsx b/src/core/public/application/integration_tests/utils.tsx
@@ -50,29 +50,39 @@ export const createAppMounter = (
   appId: string,
   html: string,
   appRoute = `/app/${appId}`
-): MockedMounterTuple<App> => [
-  appId,
-  {
-    appRoute,
-    appBasePath: appRoute,
-    mount: jest.fn(async ({ appBasePath: basename, element }: AppMountParameters) => {
-      Object.assign(element, {
-        innerHTML: `<div>\nbasename: ${basename}\nhtml: ${html}\n</div>`,
-      });
-      return jest.fn(() => Object.assign(element, { innerHTML: '' }));
-    }),
-  },
-];
+): MockedMounterTuple<App> => {
+  const unmount = jest.fn();
+  return [
+    appId,
+    {
+      mounter: {
+        appRoute,
+        appBasePath: appRoute,
+        mount: jest.fn(async ({ appBasePath: basename, element }: AppMountParameters) => {
+          Object.assign(element, {
+            innerHTML: `<div>\nbasename: ${basename}\nhtml: ${html}\n</div>`,
+          });
+          unmount.mockImplementation(() => Object.assign(element, { innerHTML: '' }));
+          return unmount;
+        }),
+      },
+      unmount,
+    },
+  ];
+};
 
 export const createLegacyAppMounter = (
   appId: string,
   legacyMount: MockedMounter<LegacyApp>['mount']
 ): MockedMounterTuple<LegacyApp> => [
   appId,
   {
-    appRoute: `/app/${appId.split(':')[0]}`,
-    appBasePath: `/app/${appId.split(':')[0]}`,
-    unmountBeforeMounting: true,
-    mount: legacyMount,
+    mounter: {
+      appRoute: `/app/${appId.split(':')[0]}`,
+      appBasePath: `/app/${appId.split(':')[0]}`,
+      unmountBeforeMounting: true,
+      mount: legacyMount,
+    },
+    unmount: jest.fn(),
   },
 ];
diff --git a/src/core/public/application/test_types.ts b/src/core/public/application/test_types.ts
@@ -17,19 +17,27 @@
  * under the License.
  */
 
-import { App, LegacyApp, Mounter } from './types';
+import { App, LegacyApp, Mounter, AppUnmount } from './types';
 import { ApplicationService } from './application_service';
 
 /** @internal */
 export type ApplicationServiceContract = PublicMethodsOf<ApplicationService>;
 /** @internal */
 export type EitherApp = App | LegacyApp;
 /** @internal */
+export type MockedUnmount = jest.Mocked<AppUnmount>;
+/** @internal */
 export type MockedMounter<T extends EitherApp> = jest.Mocked<Mounter<jest.Mocked<T>>>;
 /** @internal */
-export type MockedMounterTuple<T extends EitherApp> = [string, MockedMounter<T>];
+export type MockedMounterTuple<T extends EitherApp> = [
+  string,
+  { mounter: MockedMounter<T>; unmount: MockedUnmount }
+];
 /** @internal */
-export type MockedMounterMap<T extends EitherApp> = Map<string, MockedMounter<T>>;
+export type MockedMounterMap<T extends EitherApp> = Map<
+  string,
+  { mounter: MockedMounter<T>; unmount: MockedUnmount }
+>;
 /** @internal */
 export type MockLifecycle<
   T extends keyof ApplicationService,

diff --git a/src/core/public/application/ui/app_container.tsx b/src/core/public/application/ui/app_container.tsx
@@ -65,7 +65,7 @@ export const AppContainer: FunctionComponent<Props> = ({ mounter, appId }: Props
 
     mount();
     return unmount;
-  });
+  }, [mounter]);
 
   return (
     <Fragment>