From c75f7b4e5c7bd847fcd708e650411ec86b36f7be Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Wed, 13 May 2020 21:11:34 +0200 Subject: [PATCH] use individual route for each KP app (#65977) * use individual route for each KP app * cleanup --- .../application/application_service.tsx | 2 + .../integration_tests/router.test.tsx | 22 ++++++++++- .../application/integration_tests/utils.tsx | 2 + src/core/public/application/types.ts | 1 + .../application/ui/app_container.test.tsx | 4 +- src/core/public/application/ui/app_router.tsx | 39 +++++++++---------- 6 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/core/public/application/application_service.tsx b/src/core/public/application/application_service.tsx index fd496da26283c..6802c2363b9f8 100644 --- a/src/core/public/application/application_service.tsx +++ b/src/core/public/application/application_service.tsx @@ -198,6 +198,7 @@ export class ApplicationService { appBasePath: basePath.prepend(app.appRoute!), mount: wrapMount(plugin, app), unmountBeforeMounting: false, + legacy: false, }); }, registerLegacyApp: app => { @@ -232,6 +233,7 @@ export class ApplicationService { appBasePath, mount, unmountBeforeMounting: true, + legacy: true, }); }, registerAppUpdater: (appUpdater$: Observable) => diff --git a/src/core/public/application/integration_tests/router.test.tsx b/src/core/public/application/integration_tests/router.test.tsx index 915c58b28ad6d..9f379859dc34f 100644 --- a/src/core/public/application/integration_tests/router.test.tsx +++ b/src/core/public/application/integration_tests/router.test.tsx @@ -27,7 +27,7 @@ import { createRenderer, createAppMounter, createLegacyAppMounter, getUnmounter import { AppStatus } from '../types'; import { ScopedHistory } from '../scoped_history'; -describe('AppContainer', () => { +describe('AppRouter', () => { let mounters: MockedMounterMap; let globalHistory: History; let appStatuses$: BehaviorSubject>; @@ -78,6 +78,16 @@ describe('AppContainer', () => { history.push('/subpath'); }, }), + createAppMounter({ + appId: 'app5', + html: '
App 5
', + appRoute: '/app/my-app/app5', + }), + createAppMounter({ + appId: 'app6', + html: '
App 6
', + appRoute: '/app/my-app/app6', + }), ] as Array>); globalHistory = createMemoryHistory(); appStatuses$ = mountersToAppStatus$(); @@ -282,6 +292,16 @@ describe('AppContainer', () => { expect(unmount).not.toHaveBeenCalled(); }); + it('allows multiple apps with the same `/app/appXXX` appRoute prefix', async () => { + await navigate('/app/my-app/app5/path'); + expect(mounters.get('app5')!.mounter.mount).toHaveBeenCalledTimes(1); + expect(mounters.get('app6')!.mounter.mount).toHaveBeenCalledTimes(0); + + await navigate('/app/my-app/app6/another-path'); + expect(mounters.get('app5')!.mounter.mount).toHaveBeenCalledTimes(1); + expect(mounters.get('app6')!.mounter.mount).toHaveBeenCalledTimes(1); + }); + it('should not remount when when changing pages within app using hash history', async () => { globalHistory = createHashHistory(); update = createRenderer( diff --git a/src/core/public/application/integration_tests/utils.tsx b/src/core/public/application/integration_tests/utils.tsx index fa04b56f83ba1..6c1b81a26d63c 100644 --- a/src/core/public/application/integration_tests/utils.tsx +++ b/src/core/public/application/integration_tests/utils.tsx @@ -61,6 +61,7 @@ export const createAppMounter = ({ mounter: { appRoute, appBasePath: appRoute, + legacy: false, mount: jest.fn(async (params: AppMountParameters) => { const { appBasePath: basename, element } = params; Object.assign(element, { @@ -88,6 +89,7 @@ export const createLegacyAppMounter = ( appRoute: `/app/${appId.split(':')[0]}`, appBasePath: `/app/${appId.split(':')[0]}`, unmountBeforeMounting: true, + legacy: true, mount: legacyMount, }, unmount: jest.fn(), diff --git a/src/core/public/application/types.ts b/src/core/public/application/types.ts index 0734e178033e2..786d11a5ced7f 100644 --- a/src/core/public/application/types.ts +++ b/src/core/public/application/types.ts @@ -549,6 +549,7 @@ export type Mounter = SelectivePartial< appRoute: string; appBasePath: string; mount: T extends LegacyApp ? LegacyAppMounter : AppMounter; + legacy: boolean; unmountBeforeMounting: T extends LegacyApp ? true : boolean; }, T extends LegacyApp ? never : 'unmountBeforeMounting' diff --git a/src/core/public/application/ui/app_container.test.tsx b/src/core/public/application/ui/app_container.test.tsx index 2ee71a5bde7dc..5d573d47bd420 100644 --- a/src/core/public/application/ui/app_container.test.tsx +++ b/src/core/public/application/ui/app_container.test.tsx @@ -54,6 +54,7 @@ describe('AppContainer', () => { appBasePath: '/base-path', appRoute: '/some-route', unmountBeforeMounting: false, + legacy: false, mount: async ({ element }: AppMountParameters) => { await promise; const container = document.createElement('div'); @@ -138,9 +139,10 @@ describe('AppContainer', () => { it('should call setIsMounting(false) if mounting throws', async () => { const [waitPromise, resolvePromise] = createResolver(); const mounter = { - appBasePath: '/base-path', + appBasePath: '/base-path/some-route', appRoute: '/some-route', unmountBeforeMounting: false, + legacy: false, mount: async ({ element }: AppMountParameters) => { await waitPromise; throw new Error(`Mounting failed!`); diff --git a/src/core/public/application/ui/app_router.tsx b/src/core/public/application/ui/app_router.tsx index ea7c5c9308fe2..5d02f96134b27 100644 --- a/src/core/public/application/ui/app_router.tsx +++ b/src/core/public/application/ui/app_router.tsx @@ -55,26 +55,25 @@ export const AppRouter: FunctionComponent = ({ return ( - {[...mounters].flatMap(([appId, mounter]) => - // Remove /app paths from the routes as they will be handled by the - // "named" route parameter `:appId` below - mounter.appBasePath.startsWith('/app') - ? [] - : [ - ( - - )} - />, - ] - )} + {[...mounters] + // legacy apps can have multiple sub-apps registered with the same route + // which needs additional logic that is handled in the catch-all route below + .filter(([_, mounter]) => !mounter.legacy) + .map(([appId, mounter]) => ( + ( + + )} + /> + ))} + {/* handler for legacy apps and used as a catch-all to display 404 page on not existing /app/appId apps*/}