Skip to content

Commit

Permalink
use individual route for each KP app (#65977)
Browse files Browse the repository at this point in the history
* use individual route for each KP app

* cleanup
  • Loading branch information
pgayvallet committed May 13, 2020
1 parent 49be10c commit c75f7b4
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 22 deletions.
2 changes: 2 additions & 0 deletions src/core/public/application/application_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ export class ApplicationService {
appBasePath: basePath.prepend(app.appRoute!),
mount: wrapMount(plugin, app),
unmountBeforeMounting: false,
legacy: false,
});
},
registerLegacyApp: app => {
Expand Down Expand Up @@ -232,6 +233,7 @@ export class ApplicationService {
appBasePath,
mount,
unmountBeforeMounting: true,
legacy: true,
});
},
registerAppUpdater: (appUpdater$: Observable<AppUpdater>) =>
Expand Down
22 changes: 21 additions & 1 deletion src/core/public/application/integration_tests/router.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<EitherApp>;
let globalHistory: History;
let appStatuses$: BehaviorSubject<Map<string, AppStatus>>;
Expand Down Expand Up @@ -78,6 +78,16 @@ describe('AppContainer', () => {
history.push('/subpath');
},
}),
createAppMounter({
appId: 'app5',
html: '<div>App 5</div>',
appRoute: '/app/my-app/app5',
}),
createAppMounter({
appId: 'app6',
html: '<div>App 6</div>',
appRoute: '/app/my-app/app6',
}),
] as Array<MockedMounterTuple<EitherApp>>);
globalHistory = createMemoryHistory();
appStatuses$ = mountersToAppStatus$();
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions src/core/public/application/integration_tests/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down Expand Up @@ -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(),
Expand Down
1 change: 1 addition & 0 deletions src/core/public/application/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@ export type Mounter<T = App | LegacyApp> = SelectivePartial<
appRoute: string;
appBasePath: string;
mount: T extends LegacyApp ? LegacyAppMounter : AppMounter;
legacy: boolean;
unmountBeforeMounting: T extends LegacyApp ? true : boolean;
},
T extends LegacyApp ? never : 'unmountBeforeMounting'
Expand Down
4 changes: 3 additions & 1 deletion src/core/public/application/ui/app_container.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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!`);
Expand Down
39 changes: 19 additions & 20 deletions src/core/public/application/ui/app_router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,26 +55,25 @@ export const AppRouter: FunctionComponent<Props> = ({
return (
<Router history={history}>
<Switch>
{[...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')
? []
: [
<Route
key={mounter.appRoute}
path={mounter.appRoute}
render={({ match: { url } }) => (
<AppContainer
appPath={url}
appStatus={appStatuses.get(appId) ?? AppStatus.inaccessible}
createScopedHistory={createScopedHistory}
{...{ appId, mounter, setAppLeaveHandler, setIsMounting }}
/>
)}
/>,
]
)}
{[...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]) => (
<Route
key={mounter.appRoute}
path={mounter.appRoute}
render={({ match: { url } }) => (
<AppContainer
appPath={url}
appStatus={appStatuses.get(appId) ?? AppStatus.inaccessible}
createScopedHistory={createScopedHistory}
{...{ appId, mounter, setAppLeaveHandler, setIsMounting }}
/>
)}
/>
))}
{/* handler for legacy apps and used as a catch-all to display 404 page on not existing /app/appId apps*/}
<Route
path="/app/:appId"
render={({
Expand Down

0 comments on commit c75f7b4

Please sign in to comment.