[new-platform] only use Setup API's (#35733) (#36276)

* Core: only use Setup API's

* Fix linter issues

* Review feedback

* Update core API docs

* Make comment less coupled to Core calling code

docs/development/core/public/kibana-plugin-public.injectedmetadatasetup.getkibanabuildnumber.md

@@ -0,0 +1,11 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [InjectedMetadataSetup](./kibana-plugin-public.injectedmetadatasetup.md) &gt; [getKibanaBuildNumber](./kibana-plugin-public.injectedmetadatasetup.getkibanabuildnumber.md)
+
+## InjectedMetadataSetup.getKibanaBuildNumber property
+
+<b>Signature:</b>
+
+```typescript
+getKibanaBuildNumber: () => number;
+```

docs/development/core/public/kibana-plugin-public.injectedmetadatasetup.md

@@ -20,6 +20,7 @@ export interface InjectedMetadataSetup
 |  [getCspConfig](./kibana-plugin-public.injectedmetadatasetup.getcspconfig.md) | <code>() =&gt; {`<p/>`        warnLegacyBrowsers: boolean;`<p/>`    }</code> |  |
 |  [getInjectedVar](./kibana-plugin-public.injectedmetadatasetup.getinjectedvar.md) | <code>(name: string, defaultValue?: any) =&gt; unknown</code> |  |
 |  [getInjectedVars](./kibana-plugin-public.injectedmetadatasetup.getinjectedvars.md) | <code>() =&gt; {`<p/>`        [key: string]: unknown;`<p/>`    }</code> |  |
+|  [getKibanaBuildNumber](./kibana-plugin-public.injectedmetadatasetup.getkibanabuildnumber.md) | <code>() =&gt; number</code> |  |
 |  [getKibanaVersion](./kibana-plugin-public.injectedmetadatasetup.getkibanaversion.md) | <code>() =&gt; string</code> |  |
 |  [getLegacyMetadata](./kibana-plugin-public.injectedmetadatasetup.getlegacymetadata.md) | <code>() =&gt; {`<p/>`        app: unknown;`<p/>`        translations: unknown;`<p/>`        bundleId: string;`<p/>`        nav: LegacyNavLink[];`<p/>`        version: string;`<p/>`        branch: string;`<p/>`        buildNum: number;`<p/>`        buildSha: string;`<p/>`        basePath: string;`<p/>`        serverName: string;`<p/>`        devMode: boolean;`<p/>`        uiSettings: {`<p/>`            defaults: UiSettingsState;`<p/>`            user?: UiSettingsState &#124; undefined;`<p/>`        };`<p/>`    }</code> |  |
 |  [getPlugins](./kibana-plugin-public.injectedmetadatasetup.getplugins.md) | <code>() =&gt; Array&lt;{`<p/>`        id: string;`<p/>`        plugin: DiscoveredPlugin;`<p/>`    }&gt;</code> | An array of frontend plugins in topological order. |

src/core/public/core_system.test.ts

@@ -131,13 +131,12 @@ describe('constructor', () => {
 
     expect(FatalErrorsServiceConstructor).toHaveBeenCalledTimes(1);
 
-    expect(FatalErrorsServiceConstructor).toHaveBeenLastCalledWith({
+    expect(FatalErrorsServiceConstructor).toHaveBeenLastCalledWith(
       rootDomElement,
-      injectedMetadata: MockInjectedMetadataService,
+      expect.any(Function)
-      stopCoreSystem: expect.any(Function),
+    );
-    });
 
-    const [{ stopCoreSystem }] = FatalErrorsServiceConstructor.mock.calls[0];
+    const [, stopCoreSystem] = FatalErrorsServiceConstructor.mock.calls[0];
 
     expect(coreSystem.stop).not.toHaveBeenCalled();
     stopCoreSystem();

src/core/public/core_system.ts

@@ -22,7 +22,7 @@ import './core.css';
 import { CoreSetup, CoreStart } from '.';
 import { BasePathService } from './base_path';
 import { ChromeService } from './chrome';
-import { FatalErrorsService } from './fatal_errors';
+import { FatalErrorsService, FatalErrorsSetup } from './fatal_errors';
 import { HttpService } from './http';
 import { I18nService } from './i18n';
 import { InjectedMetadataParams, InjectedMetadataService } from './injected_metadata';
@@ -69,6 +69,7 @@ export class CoreSystem {
 
   private readonly rootDomElement: HTMLElement;
   private readonly overlayTargetDomElement: HTMLDivElement;
+  private fatalErrorsSetup: FatalErrorsSetup | null = null;
 
   constructor(params: Params) {
     const {
@@ -87,12 +88,9 @@ export class CoreSystem {
       injectedMetadata,
     });
 
-    this.fatalErrors = new FatalErrorsService({
+    this.fatalErrors = new FatalErrorsService(rootDomElement, () => {
-      rootDomElement,
+      // Stop Core before rendering any fatal errors into the DOM
-      injectedMetadata: this.injectedMetadata,
+      this.stop();
-      stopCoreSystem: () => {
-        this.stop();
-      },
     });
 
     this.notifications = new NotificationsService();
@@ -115,10 +113,13 @@ export class CoreSystem {
 
   public async setup() {
     try {
+      // Setup FatalErrorsService and it's dependencies first so that we're
+      // able to render any errors.
       const i18n = this.i18n.setup();
       const injectedMetadata = this.injectedMetadata.setup();
-      const fatalErrors = this.fatalErrors.setup({ i18n });
+      this.fatalErrorsSetup = this.fatalErrors.setup({ injectedMetadata, i18n });
-      const http = this.http.setup({ fatalErrors });
+
+      const http = this.http.setup({ fatalErrors: this.fatalErrorsSetup });
       const basePath = this.basePath.setup({ injectedMetadata });
       const uiSettings = this.uiSettings.setup({
         http,
@@ -136,7 +137,7 @@ export class CoreSystem {
         application,
         basePath,
         chrome,
-        fatalErrors,
+        fatalErrors: this.fatalErrorsSetup,
         http,
         i18n,
         injectedMetadata,
@@ -148,9 +149,15 @@ export class CoreSystem {
       await this.plugins.setup(core);
       await this.legacyPlatform.setup({ core });
 
-      return { fatalErrors };
+      return { fatalErrors: this.fatalErrorsSetup };
     } catch (error) {
-      this.fatalErrors.add(error);
+      if (this.fatalErrorsSetup) {
+        this.fatalErrorsSetup.add(error);
+      } else {
+        // If the FatalErrorsService has not yet been setup, log error to console
+        // eslint-disable-next-line no-console
+        console.log(error);
+      }
     }
   }
 
@@ -189,7 +196,13 @@ export class CoreSystem {
       await this.plugins.start(core);
       await this.legacyPlatform.start({ core, targetDomElement: legacyPlatformTargetDomElement });
     } catch (error) {
-      this.fatalErrors.add(error);
+      if (this.fatalErrorsSetup) {
+        this.fatalErrorsSetup.add(error);
+      } else {
+        // If the FatalErrorsService has not yet been setup, log error to console
+        // eslint-disable-next-line no-console
+        console.error(error);
+      }
     }
   }
 

src/core/public/fatal_errors/__snapshots__/fatal_errors_service.test.ts.snap

@@ -3,13 +3,12 @@
 exports[`#add() deletes all children of rootDomElement and renders <FatalErrorScreen /> into it: fatal error screen component 1`] = `
 Array [
   Array [
-    <React.Fragment>
+    <I18nContext>
       <FatalErrorsScreen
-        buildNumber="kibanaBuildNumber"
         errorInfo$={Rx.Observable}
         kibanaVersion="kibanaVersion"
       />
-    </React.Fragment>,
+    </I18nContext>,
     <div />,
   ],
 ]
@@ -20,24 +19,3 @@ exports[`#add() deletes all children of rootDomElement and renders <FatalErrorSc
   <div />
 </div>
 `;
-
-exports[`setup.add() deletes all children of rootDomElement and renders <FatalErrorScreen /> into it: fatal error screen component 1`] = `
-Array [
-  Array [
-    <I18nContext>
-      <FatalErrorsScreen
-        buildNumber="kibanaBuildNumber"
-        errorInfo$={Rx.Observable}
-        kibanaVersion="kibanaVersion"
-      />
-    </I18nContext>,
-    <div />,
-  ],
-]
-`;
-
-exports[`setup.add() deletes all children of rootDomElement and renders <FatalErrorScreen /> into it: fatal error screen container 1`] = `
-<div>
-  <div />
-</div>
-`;

src/core/public/fatal_errors/fatal_errors_service.mock.ts

@@ -31,7 +31,6 @@ type FatalErrorsServiceContract = PublicMethodsOf<FatalErrorsService>;
 const createMock = () => {
   const mocked: jest.Mocked<FatalErrorsServiceContract> = {
     setup: jest.fn(),
-    add: jest.fn<never, any>(() => undefined as never),
   };
 
   mocked.setup.mockReturnValue(createSetupContractMock());

src/core/public/fatal_errors/fatal_errors_service.test.ts

@@ -25,16 +25,14 @@ expect.addSnapshotSerializer({
 });
 
 import { mockRender } from './fatal_errors_service.test.mocks';
+import { injectedMetadataServiceMock } from '../injected_metadata/injected_metadata_service.mock';
 
 import { FatalErrorsService } from './fatal_errors_service';
 
 function setupService() {
   const rootDomElement = document.createElement('div');
 
-  const injectedMetadata = {
+  const injectedMetadata = injectedMetadataServiceMock.createSetupContract();
-    getKibanaBuildNumber: jest.fn().mockReturnValue('kibanaBuildNumber'),
-    getKibanaVersion: jest.fn().mockReturnValue('kibanaVersion'),
-  };
 
   const stopCoreSystem = jest.fn();
 
@@ -44,16 +42,13 @@ function setupService() {
     },
   };
 
+  const fatalErrorsService = new FatalErrorsService(rootDomElement, stopCoreSystem);
+
   return {
     rootDomElement,
     injectedMetadata,
-    i18n,
     stopCoreSystem,
-    fatalErrors: new FatalErrorsService({
+    fatalErrors: fatalErrorsService.setup({ injectedMetadata, i18n }),
-      injectedMetadata: injectedMetadata as any,
-      rootDomElement,
-      stopCoreSystem,
-    }),
   };
 }
 
@@ -91,46 +86,12 @@ describe('#add()', () => {
   });
 });
 
-describe('setup.add()', () => {
-  it('exposes a function that passes its two arguments to fatalErrors.add()', () => {
-    const { fatalErrors, i18n } = setupService();
-
-    jest.spyOn(fatalErrors, 'add').mockImplementation(() => undefined as never);
-
-    expect(fatalErrors.add).not.toHaveBeenCalled();
-    const { add } = fatalErrors.setup({ i18n });
-    add('foo', 'bar');
-    expect(fatalErrors.add).toHaveBeenCalledTimes(1);
-    expect(fatalErrors.add).toHaveBeenCalledWith('foo', 'bar');
-  });
-
-  it('deletes all children of rootDomElement and renders <FatalErrorScreen /> into it', () => {
-    const { fatalErrors, i18n, rootDomElement } = setupService();
-
-    rootDomElement.innerHTML = `
-      <h1>Loading...</h1>
-      <div class="someSpinner"></div>
-    `;
-
-    expect(mockRender).not.toHaveBeenCalled();
-    expect(rootDomElement.children).toHaveLength(2);
-
-    const { add } = fatalErrors.setup({ i18n });
-
-    expect(() => add(new Error('foo'))).toThrowError();
-    expect(rootDomElement).toMatchSnapshot('fatal error screen container');
-    expect(mockRender.mock.calls).toMatchSnapshot('fatal error screen component');
-  });
-});
-
 describe('setup.get$()', () => {
   it('provides info about the errors passed to fatalErrors.add()', () => {
-    const { fatalErrors, i18n } = setupService();
+    const { fatalErrors } = setupService();
-
-    const setup = fatalErrors.setup({ i18n });
 
     const onError = jest.fn();
-    setup.get$().subscribe(onError);
+    fatalErrors.get$().subscribe(onError);
 
     expect(onError).not.toHaveBeenCalled();
     expect(() => {

src/core/public/fatal_errors/fatal_errors_service.tsx

@@ -23,18 +23,13 @@ import * as Rx from 'rxjs';
 import { first, tap } from 'rxjs/operators';
 
 import { I18nSetup } from '../i18n';
-import { InjectedMetadataService } from '../injected_metadata';
+import { InjectedMetadataSetup } from '../';
 import { FatalErrorsScreen } from './fatal_errors_screen';
 import { ErrorInfo, getErrorInfo } from './get_error_info';
 
-export interface FatalErrorsParams {
-  rootDomElement: HTMLElement;
-  injectedMetadata: InjectedMetadataService;
-  stopCoreSystem: () => void;
-}
-
 interface Deps {
   i18n: I18nSetup;
+  injectedMetadata: InjectedMetadataSetup;
 }
 
 /**
@@ -62,41 +57,45 @@ export interface FatalErrorsSetup {
 /** @interal */
 export class FatalErrorsService {
   private readonly errorInfo$ = new Rx.ReplaySubject<ErrorInfo>();
-  private i18n?: I18nSetup;
 
-  constructor(private params: FatalErrorsParams) {
+  /**
+   *
+   * @param rootDomElement
+   * @param onFirstErrorCb - Callback function that gets executed after the first error,
+   *   but before the FatalErrorsService renders the error to the DOM.
+   */
+  constructor(private rootDomElement: HTMLElement, private onFirstErrorCb: () => void) {}
+
+  public setup({ i18n, injectedMetadata }: Deps) {
     this.errorInfo$
       .pipe(
         first(),
-        tap(() => this.onFirstError())
+        tap(() => {
+          this.onFirstErrorCb();
+          this.renderError(injectedMetadata, i18n);
+        })
       )
       .subscribe({
         error: error => {
           // eslint-disable-next-line no-console
-          console.error('Uncaught error in fatal error screen internals', error);
+          console.error('Uncaught error in fatal error service internals', error);
         },
       });
-  }
-
-  public add: FatalErrorsSetup['add'] = (error, source?) => {
-    const errorInfo = getErrorInfo(error, source);
-
-    this.errorInfo$.next(errorInfo);
-
-    if (error instanceof Error) {
-      // make stack traces clickable by putting whole error in the console
-      // eslint-disable-next-line no-console
-      console.error(error);
-    }
-
-    throw error;
-  };
-
-  public setup({ i18n }: Deps) {
-    this.i18n = i18n;
 
     const fatalErrorsSetup: FatalErrorsSetup = {
-      add: this.add,
+      add: (error, source?) => {
+        const errorInfo = getErrorInfo(error, source);
+
+        this.errorInfo$.next(errorInfo);
+
+        if (error instanceof Error) {
+          // make stack traces clickable by putting whole error in the console
+          // eslint-disable-next-line no-console
+          console.error(error);
+        }
+
+        throw error;
+      },
       get$: () => {
         return this.errorInfo$.asObservable();
       },
@@ -105,30 +104,22 @@ export class FatalErrorsService {
     return fatalErrorsSetup;
   }
 
-  private onFirstError() {
+  private renderError(injectedMetadata: InjectedMetadataSetup, i18n: I18nSetup) {
-    // stop the core systems so that things like the legacy platform are stopped
-    // and angular/react components are unmounted;
-    this.params.stopCoreSystem();
-
     // delete all content in the rootDomElement
-    this.params.rootDomElement.textContent = '';
+    this.rootDomElement.textContent = '';
 
     // create and mount a container for the <FatalErrorScreen>
     const container = document.createElement('div');
-    this.params.rootDomElement.appendChild(container);
+    this.rootDomElement.appendChild(container);
-
-    // If error occurred before I18nService has been set up we don't have any
-    // i18n context to provide.
-    const I18nContext = this.i18n ? this.i18n.Context : React.Fragment;
 
     render(
-      <I18nContext>
+      <i18n.Context>
         <FatalErrorsScreen
-          buildNumber={this.params.injectedMetadata.getKibanaBuildNumber()}
+          buildNumber={injectedMetadata.getKibanaBuildNumber()}
-          kibanaVersion={this.params.injectedMetadata.getKibanaVersion()}
+          kibanaVersion={injectedMetadata.getKibanaVersion()}
           errorInfo$={this.errorInfo$}
         />
-      </I18nContext>,
+      </i18n.Context>,
       container
     );
   }

src/core/public/injected_metadata/injected_metadata_service.mock.ts

@@ -27,6 +27,7 @@ const createSetupContractMock = () => {
     getPlugins: jest.fn(),
     getInjectedVar: jest.fn(),
     getInjectedVars: jest.fn(),
+    getKibanaBuildNumber: jest.fn(),
   };
   setupContract.getCspConfig.mockReturnValue({ warnLegacyBrowsers: true });
   setupContract.getKibanaVersion.mockReturnValue('kibanaVersion');
@@ -47,8 +48,6 @@ type InjectedMetadataServiceContract = PublicMethodsOf<InjectedMetadataService>;
 const createMock = (): jest.Mocked<InjectedMetadataServiceContract> => ({
   setup: jest.fn().mockReturnValue(createSetupContractMock()),
   start: jest.fn().mockReturnValue(createStartContractMock()),
-  getKibanaVersion: jest.fn(),
-  getKibanaBuildNumber: jest.fn(),
 });
 
 export const injectedMetadataServiceMock = {

src/core/public/injected_metadata/injected_metadata_service.test.ts

@@ -20,42 +20,29 @@
 import { DiscoveredPlugin } from '../../server';
 import { InjectedMetadataService } from './injected_metadata_service';
 
-describe('#getKibanaVersion', () => {
+describe('setup.getKibanaBuildNumber()', () => {
-  it('returns version from injectedMetadata', () => {
-    const injectedMetadata = new InjectedMetadataService({
-      injectedMetadata: {
-        version: 'foo',
-      },
-    } as any);
-
-    expect(injectedMetadata.getKibanaVersion()).toBe('foo');
-  });
-});
-
-describe('#getKibanaBuildNumber', () => {
   it('returns buildNumber from injectedMetadata', () => {
-    const injectedMetadata = new InjectedMetadataService({
+    const setup = new InjectedMetadataService({
       injectedMetadata: {
         buildNumber: 'foo',
       },
-    } as any);
+    } as any).setup();
 
-    expect(injectedMetadata.getKibanaBuildNumber()).toBe('foo');
+    expect(setup.getKibanaBuildNumber()).toBe('foo');
   });
 });
 
 describe('setup.getCspConfig()', () => {
   it('returns injectedMetadata.csp', () => {
-    const injectedMetadata = new InjectedMetadataService({
+    const setup = new InjectedMetadataService({
       injectedMetadata: {
         csp: {
           warnLegacyBrowsers: true,
         },
       },
-    } as any);
+    } as any).setup();
 
-    const contract = injectedMetadata.setup();
+    expect(setup.getCspConfig()).toEqual({
-    expect(contract.getCspConfig()).toEqual({
       warnLegacyBrowsers: true,
     });
   });

src/core/public/injected_metadata/injected_metadata_service.ts

@@ -83,6 +83,10 @@ export class InjectedMetadataService {
 
   constructor(private readonly params: InjectedMetadataParams) {}
 
+  public start(): InjectedMetadataStart {
+    return this.setup();
+  }
+
   public setup(): InjectedMetadataSetup {
     return {
       getBasePath: () => {
@@ -90,7 +94,7 @@ export class InjectedMetadataService {
       },
 
       getKibanaVersion: () => {
-        return this.getKibanaVersion();
+        return this.state.version;
       },
 
       getCspConfig: () => {
@@ -112,20 +116,12 @@ export class InjectedMetadataService {
       getInjectedVars: () => {
         return this.state.vars;
       },
+
+      getKibanaBuildNumber: () => {
+        return this.state.buildNumber;
+      },
     };
   }
-
-  public start(): InjectedMetadataStart {
-    return this.setup();
-  }
-
-  public getKibanaVersion() {
-    return this.state.version;
-  }
-
-  public getKibanaBuildNumber() {
-    return this.state.buildNumber;
-  }
 }
 
 /**
@@ -135,6 +131,7 @@ export class InjectedMetadataService {
  */
 export interface InjectedMetadataSetup {
   getBasePath: () => string;
+  getKibanaBuildNumber: () => number;
   getKibanaVersion: () => string;
   getCspConfig: () => {
     warnLegacyBrowsers: boolean;

src/core/public/public.api.md

@@ -142,7 +142,7 @@ export class CoreSystem {
     constructor(params: Params);
     // (undocumented)
     setup(): Promise<{
-        fatalErrors: import(".").FatalErrorsSetup;
+        fatalErrors: FatalErrorsSetup;
     } | undefined>;
     // (undocumented)
     start(): Promise<void>;
@@ -233,6 +233,8 @@ export interface InjectedMetadataSetup {
         [key: string]: unknown;
     };
     // (undocumented)
+    getKibanaBuildNumber: () => number;
+    // (undocumented)
     getKibanaVersion: () => string;
     // (undocumented)
     getLegacyMetadata: () => {