From 97c86a364c7c4e9040dd0dfa34d9e14add60c8ef Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 11 Jul 2018 13:35:40 -0700 Subject: [PATCH 01/30] [core/ui] bootstrap the legacy platform within the new platform --- src/core/public/core_system.ts | 50 +++++++++++++ src/core/public/index.ts | 20 +++++ .../public/injected_metadata/deep_freeze.ts | 36 +++++++++ src/core/public/injected_metadata/index.ts | 20 +++++ .../injected_metadata_service.ts} | 40 ++++------ .../read_injected_metadata_from_dom.ts | 33 +++++++++ src/core_plugins/tests_bundle/index.js | 12 +-- .../tests_bundle/tests_entry_template.js | 16 +++- src/optimize/index.js | 8 +- src/optimize/watch/watch_optimizer.js | 5 +- src/ui/public/chrome/chrome.js | 3 - src/ui/public/metadata.js | 29 ++------ src/ui/public/test_harness/index.js | 2 - src/ui/ui_apps/__tests__/ui_app.js | 20 +++-- src/ui/ui_apps/ui_app.js | 4 +- ...late.js => new_platform_entry_template.js} | 13 ++-- src/ui/ui_bundles/ui_bundle.js | 21 ++---- src/ui/ui_bundles/ui_bundles_controller.js | 74 +++++++++---------- src/ui/ui_exports/ui_export_defaults.js | 2 +- src/ui/ui_render/ui_render_mixin.js | 22 +++--- src/ui/ui_render/views/chrome.jade | 48 ++++++------ src/ui/ui_render/views/ui_app.jade | 2 +- 22 files changed, 297 insertions(+), 183 deletions(-) create mode 100644 src/core/public/core_system.ts create mode 100644 src/core/public/index.ts create mode 100644 src/core/public/injected_metadata/deep_freeze.ts create mode 100644 src/core/public/injected_metadata/index.ts rename src/{ui/ui_bundles/__tests__/app_entry_template.js => core/public/injected_metadata/injected_metadata_service.ts} (51%) create mode 100644 src/core/public/injected_metadata/read_injected_metadata_from_dom.ts rename src/ui/ui_bundles/{app_entry_template.js => new_platform_entry_template.js} (77%) diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts new file mode 100644 index 0000000000000..3f196a3b1ea5e --- /dev/null +++ b/src/core/public/core_system.ts @@ -0,0 +1,50 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// import global polyfills before everything else +import 'babel-polyfill'; +import 'custom-event-polyfill'; +import 'whatwg-fetch'; + +import { InjectedMetadata, InjectedMetadataService } from './injected_metadata'; + +interface CoreSystemParams { + injectedMetadataForTesting?: InjectedMetadata; +} + +export class CoreSystem { + private injectedMetadata: InjectedMetadataService; + + constructor(params: CoreSystemParams = {}) { + const { injectedMetadataForTesting } = params; + + this.injectedMetadata = new InjectedMetadataService(injectedMetadataForTesting); + } + + public initLegacyPlatform(bootstrapFn: () => void) { + /** + * Injects parts of the new platform into parts of the legacy platform + * so that legacy APIs/modules can mimic their new platform counterparts + */ + require('ui/metadata').__newPlatformInit__(this.injectedMetadata.getLegacyMetadata()); + + // call the legacy platform bootstrap function (bootstraps ui/chrome in apps and ui/test_harness in browser tests) + bootstrapFn(); + } +} diff --git a/src/core/public/index.ts b/src/core/public/index.ts new file mode 100644 index 0000000000000..fcf7ffd908a47 --- /dev/null +++ b/src/core/public/index.ts @@ -0,0 +1,20 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export { CoreSystem } from './core_system'; diff --git a/src/core/public/injected_metadata/deep_freeze.ts b/src/core/public/injected_metadata/deep_freeze.ts new file mode 100644 index 0000000000000..45bdd93117c5d --- /dev/null +++ b/src/core/public/injected_metadata/deep_freeze.ts @@ -0,0 +1,36 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +type Freezable = { [k: string]: any } | ArrayLike; + +type RecursiveReadOnly = T extends Freezable + ? Readonly<{ [K in keyof T]: RecursiveReadOnly }> + : T; + +export function deepFreeze(object: T) { + // for any properties that reference an object, makes sure that object is + // recursively frozen as well + for (const value of Object.values(object)) { + if (value !== null && typeof value === 'object') { + deepFreeze(value); + } + } + + return Object.freeze(object) as RecursiveReadOnly; +} diff --git a/src/core/public/injected_metadata/index.ts b/src/core/public/injected_metadata/index.ts new file mode 100644 index 0000000000000..662fb7f3033a8 --- /dev/null +++ b/src/core/public/injected_metadata/index.ts @@ -0,0 +1,20 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export { InjectedMetadata, InjectedMetadataService } from './injected_metadata_service'; diff --git a/src/ui/ui_bundles/__tests__/app_entry_template.js b/src/core/public/injected_metadata/injected_metadata_service.ts similarity index 51% rename from src/ui/ui_bundles/__tests__/app_entry_template.js rename to src/core/public/injected_metadata/injected_metadata_service.ts index c026c3b759052..fcc38b896dce7 100644 --- a/src/ui/ui_bundles/__tests__/app_entry_template.js +++ b/src/core/public/injected_metadata/injected_metadata_service.ts @@ -17,33 +17,23 @@ * under the License. */ -import sinon from 'sinon'; -import expect from 'expect.js'; +import { deepFreeze } from './deep_freeze'; +import { readInjectedMetadataFromDom } from './read_injected_metadata_from_dom'; -import { appEntryTemplate } from '../app_entry_template'; - -function createMockBundle() { - return { - getContext: sinon.stub().returns(''), - getRequires: sinon.stub().returns([]) +export interface InjectedState { + legacyMetadata: { + [key: string]: any; }; } -describe('ui bundles / appEntryTemplate', () => { - it('embeds bundle.getContext() result', () => { - const bundle = createMockBundle(); - bundle.getContext.returns('foo bar baz'); - expect(appEntryTemplate(bundle)).to.contain('foo bar baz'); - }); +export class InjectedStateService { + private state: InjectedState; + + constructor(injectedStateForTesting?: InjectedState) { + this.state = deepFreeze(injectedStateForTesting || readInjectedMetadataFromDom()); + } - it('joins requires into list', () => { - const bundle = createMockBundle(); - const requires = [ - 'foo', - 'bar', - 'baz' - ]; - bundle.getRequires.returns(requires); - expect(appEntryTemplate(bundle)).to.contain(requires.join('\n')); - }); -}); + public getLegacyMetadata() { + return this.state.legacyMetadata; + } +} diff --git a/src/core/public/injected_metadata/read_injected_metadata_from_dom.ts b/src/core/public/injected_metadata/read_injected_metadata_from_dom.ts new file mode 100644 index 0000000000000..a903777ba507c --- /dev/null +++ b/src/core/public/injected_metadata/read_injected_metadata_from_dom.ts @@ -0,0 +1,33 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export function readInjectedMetadataFromDom() { + const el = document.querySelector('kbn-injected-metadata'); + + if (!el) { + throw new Error('unable to find element'); + } + + const json = el.getAttribute('data'); + if (!json) { + throw new Error(' does not have a data atttibute'); + } + + return JSON.parse(json); +} diff --git a/src/core_plugins/tests_bundle/index.js b/src/core_plugins/tests_bundle/index.js index b18de4b0dc6da..bc9c736a0311a 100644 --- a/src/core_plugins/tests_bundle/index.js +++ b/src/core_plugins/tests_bundle/index.js @@ -17,8 +17,6 @@ * under the License. */ -import { union } from 'lodash'; - import { fromRoot } from '../../utils'; import findSourceFiles from './find_source_files'; @@ -36,7 +34,7 @@ export default (kibana) => { uiExports: { async __bundleProvider__(kbnServer) { - let modules = []; + const modules = []; const { config, @@ -66,8 +64,8 @@ export default (kibana) => { // add the modules from all of this plugins apps for (const app of uiApps) { - if (app.getPluginId() === pluginId) { - modules = union(modules, app.getModules()); + if (app.getPluginId() === pluginId && !modules.includes(app.getMainModuleId())) { + modules.push(app.getMainModuleId()); } } @@ -76,7 +74,9 @@ export default (kibana) => { } else { // add the modules from all of the apps for (const app of uiApps) { - modules = union(modules, app.getModules()); + if (!modules.includes(app.getMainModuleId())) { + modules.push(app.getMainModuleId()); + } } for (const plugin of plugins) { diff --git a/src/core_plugins/tests_bundle/tests_entry_template.js b/src/core_plugins/tests_bundle/tests_entry_template.js index 32891f98a1ec1..31efa5b626afc 100644 --- a/src/core_plugins/tests_bundle/tests_entry_template.js +++ b/src/core_plugins/tests_bundle/tests_entry_template.js @@ -29,7 +29,9 @@ export const createTestEntryTemplate = (defaultUiSettings) => (bundle) => ` * */ -window.__KBN__ = { +import { CoreSystem } from '__kibanaCore__' + +const legacyMetadata = { version: '1.2.3', buildNum: 1234, vars: { @@ -64,7 +66,13 @@ window.__KBN__ = { } }; -require('ui/test_harness'); -${bundle.getRequires().join('\n')} -require('ui/test_harness').bootstrap(/* go! */); +new CoreSystem({ + injectedMetadataForTesting: { + legacyMetadata + } +}).initLegacyPlatform(() => { + require('ui/test_harness'); + ${bundle.getRequires().join('\n ')} + require('ui/test_harness').bootstrap(); +}) `; diff --git a/src/optimize/index.js b/src/optimize/index.js index 4ed117730de35..8675a4fd2e458 100644 --- a/src/optimize/index.js +++ b/src/optimize/index.js @@ -42,12 +42,6 @@ export default async (kbnServer, server, config) => { basePublicPath: config.get('server.basePath') })); - await uiBundles.writeEntryFiles(); - - // Not all entry files produce a css asset. Ensuring they exist prevents - // an error from occurring when the file is missing. - await uiBundles.ensureStyleFiles(); - // in prod, only bundle when something is missing or invalid const reuseCache = config.get('optimize.useBundleCache') ? await uiBundles.areAllBundleCachesValid() @@ -62,6 +56,8 @@ export default async (kbnServer, server, config) => { return; } + await uiBundles.resetBundleDir(); + // only require the FsOptimizer when we need to const optimizer = new FsOptimizer({ uiBundles, diff --git a/src/optimize/watch/watch_optimizer.js b/src/optimize/watch/watch_optimizer.js index ea07025301ce4..215361a8cbf58 100644 --- a/src/optimize/watch/watch_optimizer.js +++ b/src/optimize/watch/watch_optimizer.js @@ -45,10 +45,7 @@ export default class WatchOptimizer extends BaseOptimizer { // log status changes this.status$.subscribe(this.onStatusChangeHandler); - - await this.uiBundles.writeEntryFiles(); - await this.uiBundles.ensureStyleFiles(); - + await this.uiBundles.resetBundleDir(); await this.initCompiler(); this.compiler.plugin('watch-run', this.compilerRunStartHandler); diff --git a/src/ui/public/chrome/chrome.js b/src/ui/public/chrome/chrome.js index f1c02e870b3db..fcd413602f3d5 100644 --- a/src/ui/public/chrome/chrome.js +++ b/src/ui/public/chrome/chrome.js @@ -21,9 +21,6 @@ import _ from 'lodash'; import angular from 'angular'; import { metadata } from '../metadata'; -import 'babel-polyfill'; -import 'whatwg-fetch'; -import 'custom-event-polyfill'; import '../state_management/global_state'; import '../config'; import '../notify'; diff --git a/src/ui/public/metadata.js b/src/ui/public/metadata.js index ed65aec83fed5..6701475d947d0 100644 --- a/src/ui/public/metadata.js +++ b/src/ui/public/metadata.js @@ -17,29 +17,12 @@ * under the License. */ -import $ from 'jquery'; -import _ from 'lodash'; +export let metadata = null; -export const metadata = deepFreeze(getState()); - -function deepFreeze(object) { - // for any properties that reference an object, makes sure that object is - // recursively frozen as well - Object.keys(object).forEach(key => { - const value = object[key]; - if (_.isObject(value)) { - deepFreeze(value); - } - }); - - return Object.freeze(object); -} - -function getState() { - const stateKey = '__KBN__'; - if (!(stateKey in window)) { - const state = $('kbn-initial-state').attr('data'); - window[stateKey] = JSON.parse(state); +export function __newPlatformInit__(legacyMetadata) { + if (metadata === null) { + metadata = legacyMetadata; + } else { + throw new Error('ui/metadata can only be initialized once'); } - return window[stateKey]; } diff --git a/src/ui/public/test_harness/index.js b/src/ui/public/test_harness/index.js index beb22897d90df..d66a4b1d67214 100644 --- a/src/ui/public/test_harness/index.js +++ b/src/ui/public/test_harness/index.js @@ -17,6 +17,4 @@ * under the License. */ -import './test_harness'; - export { bootstrap } from './test_harness'; diff --git a/src/ui/ui_apps/__tests__/ui_app.js b/src/ui/ui_apps/__tests__/ui_app.js index 7d121087a2ec0..87b57a556ca4f 100644 --- a/src/ui/ui_apps/__tests__/ui_app.js +++ b/src/ui/ui_apps/__tests__/ui_app.js @@ -86,8 +86,8 @@ describe('ui apps / UiApp', () => { expect(app.getNavLink()).to.be.a(UiNavLink); }); - it('has an empty modules list', () => { - expect(app.getModules()).to.eql([]); + it('has no main module', () => { + expect(app.getMainModuleId()).to.be(undefined); }); it('has no styleSheetPath', () => { @@ -135,10 +135,8 @@ describe('ui apps / UiApp', () => { expect(app.getNavLink()).to.be(undefined); }); - it('includes main and hack modules', () => { - expect(app.getModules()).to.eql([ - 'main.js', - ]); + it('has a main module', () => { + expect(app.getMainModuleId()).to.be('main.js'); }); it('has spec values in JSON representation', () => { @@ -303,15 +301,15 @@ describe('ui apps / UiApp', () => { }); }); - describe('#getModules', () => { - it('returns empty array by default', () => { + describe('#getMainModuleId', () => { + it('returns undefined by default', () => { const app = createUiApp({ id: 'foo' }); - expect(app.getModules()).to.eql([]); + expect(app.getMainModuleId()).to.be(undefined); }); - it('returns main module if not using appExtensions', () => { + it('returns main module id', () => { const app = createUiApp({ id: 'foo', main: 'bar' }); - expect(app.getModules()).to.eql(['bar']); + expect(app.getMainModuleId()).to.be('bar'); }); }); diff --git a/src/ui/ui_apps/ui_app.js b/src/ui/ui_apps/ui_app.js index a6621bcf1408b..2469f9717d63e 100644 --- a/src/ui/ui_apps/ui_app.js +++ b/src/ui/ui_apps/ui_app.js @@ -101,8 +101,8 @@ export class UiApp { } } - getModules() { - return this._main ? [this._main] : []; + getMainModuleId() { + return this._main; } getStyleSheetUrlPath() { diff --git a/src/ui/ui_bundles/app_entry_template.js b/src/ui/ui_bundles/new_platform_entry_template.js similarity index 77% rename from src/ui/ui_bundles/app_entry_template.js rename to src/ui/ui_bundles/new_platform_entry_template.js index 4c9dca0a3368b..e7ef5c8d02bdc 100644 --- a/src/ui/ui_bundles/app_entry_template.js +++ b/src/ui/ui_bundles/new_platform_entry_template.js @@ -17,17 +17,20 @@ * under the License. */ -export const appEntryTemplate = (bundle) => ` +export const newPlatformEntryTemplate = (bundle) => ` /** - * Test entry file + * Kibana entry file * * This is programmatically created and updated, do not modify * * context: ${bundle.getContext()} */ -require('ui/chrome'); -${bundle.getRequires().join('\n')} -require('ui/chrome').bootstrap(/* xoxo */); +import { CoreSystem } from '__kibanaCore__' +new CoreSystem().initLegacyPlatform(() => { + require('ui/chrome') + ${bundle.getRequires().join('\n ')} + require('ui/chrome').bootstrap(); +}) `; diff --git a/src/ui/ui_bundles/ui_bundle.js b/src/ui/ui_bundles/ui_bundle.js index 34aeaa2a43994..015305bba0d3d 100644 --- a/src/ui/ui_bundles/ui_bundle.js +++ b/src/ui/ui_bundles/ui_bundle.js @@ -18,7 +18,7 @@ */ import { fromNode as fcb } from 'bluebird'; -import { readFile, writeFile, unlink, stat } from 'fs'; +import { readFile, writeFile, stat } from 'fs'; // We normalize all path separators to `/` in generated files function normalizePath(path) { @@ -86,31 +86,20 @@ export class UiBundle { )); } - async hasStyleFile() { - return await fcb(cb => { - return stat(this.getStylePath(), error => { - cb(null, !(error && error.code === 'ENOENT')); - }); - }); - } - async touchStyleFile() { return await fcb(cb => ( writeFile(this.getStylePath(), '', 'utf8', cb) )); } - async clearBundleFile() { - try { - await fcb(cb => unlink(this.getOutputPath(), cb)); - } catch (e) { - return null; + async isCacheValid() { + if (await this.readEntryFile() !== this.renderContent()) { + return false; } - } - async isCacheValid() { try { await fcb(cb => stat(this.getOutputPath(), cb)); + await fcb(cb => stat(this.getStylePath(), cb)); return true; } catch (e) { diff --git a/src/ui/ui_bundles/ui_bundles_controller.js b/src/ui/ui_bundles/ui_bundles_controller.js index 3a4b1385da4af..0a890393ee78c 100644 --- a/src/ui/ui_bundles/ui_bundles_controller.js +++ b/src/ui/ui_bundles/ui_bundles_controller.js @@ -17,14 +17,19 @@ * under the License. */ -import { createHash } from 'crypto'; import { resolve } from 'path'; +import { createHash } from 'crypto'; +import { promisify } from 'util'; +import { existsSync } from 'fs'; -import { UiBundle } from './ui_bundle'; -import { fromNode as fcb } from 'bluebird'; +import del from 'del'; import { makeRe } from 'minimatch'; import mkdirp from 'mkdirp'; -import { appEntryTemplate } from './app_entry_template'; + +import { UiBundle } from './ui_bundle'; +import { newPlatformEntryTemplate } from './new_platform_entry_template'; + +const mkdirpAsync = promisify(mkdirp); function getWebpackAliases(pluginSpecs) { return pluginSpecs.reduce((aliases, spec) => { @@ -74,11 +79,12 @@ export class UiBundlesController { this._postLoaders = []; this._bundles = []; + // create a bundle for each uiApp for (const uiApp of uiApps) { this.add({ id: uiApp.getId(), - modules: uiApp.getModules(), - template: appEntryTemplate, + modules: [uiApp.getMainModuleId()], + template: newPlatformEntryTemplate, }); } } @@ -140,58 +146,48 @@ export class UiBundlesController { return resolve(this._workingDir, ...args); } + async resetBundleDir() { + if (!existsSync(this._workingDir)) { + // create a fresh working directory + await mkdirpAsync(this._workingDir); + } else { + // delete all children of the working directory + await del(this.resolvePath('*')); + } + + // write the entry/style files for each bundle + for (const bundle of this._bundles) { + await bundle.writeEntryFile(); + await bundle.touchStyleFile(); + } + } + getCacheDirectory(...subPath) { return this.resolvePath('../.cache', this.hashBundleEntries(), ...subPath); } getDescription() { - switch (this._bundles.length) { + const ids = this.getIds(); + switch (ids.length) { case 0: return '0 bundles'; case 1: - return `bundle for ${this._bundles[0].getId()}`; + return `bundle for ${ids[0]}`; default: - const ids = this.getIds(); const last = ids.pop(); const commas = ids.join(', '); return `bundles for ${commas} and ${last}`; } } - async ensureDir() { - await fcb(cb => mkdirp(this._workingDir, cb)); - } - - async writeEntryFiles() { - await this.ensureDir(); - - for (const bundle of this._bundles) { - const existing = await bundle.readEntryFile(); - const expected = bundle.renderContent(); - - if (existing !== expected) { - await bundle.writeEntryFile(); - await bundle.clearBundleFile(); - } - } - } - - async ensureStyleFiles() { - await this.ensureDir(); - - for (const bundle of this._bundles) { - if (!await bundle.hasStyleFile()) { - await bundle.touchStyleFile(); - } - } - } - hashBundleEntries() { const hash = createHash('sha1'); + for (const bundle of this._bundles) { hash.update(`bundleEntryPath:${bundle.getEntryPath()}`); hash.update(`bundleEntryContent:${bundle.renderContent()}`); } + return hash.digest('hex'); } @@ -216,8 +212,4 @@ export class UiBundlesController { return this._bundles .map(bundle => bundle.getId()); } - - toJSON() { - return this._bundles; - } } diff --git a/src/ui/ui_exports/ui_export_defaults.js b/src/ui/ui_exports/ui_export_defaults.js index ae8f175d0e846..1a482b291d808 100644 --- a/src/ui/ui_exports/ui_export_defaults.js +++ b/src/ui/ui_exports/ui_export_defaults.js @@ -28,7 +28,7 @@ export const UI_EXPORT_DEFAULTS = { webpackAliases: { ui: resolve(ROOT, 'src/ui/public'), - ui_framework: resolve(ROOT, 'ui_framework'), + '__kibanaCore__$': resolve(ROOT, 'src/core/public'), test_harness: resolve(ROOT, 'src/test_harness/public'), querystring: 'querystring-browser', moment$: resolve(ROOT, 'webpackShims/moment'), diff --git a/src/ui/ui_render/ui_render_mixin.js b/src/ui/ui_render/ui_render_mixin.js index 5990f1422200d..8385f129c12c4 100644 --- a/src/ui/ui_render/ui_render_mixin.js +++ b/src/ui/ui_render/ui_render_mixin.js @@ -106,7 +106,7 @@ export function uiRenderMixin(kbnServer, server, config) { } }); - async function getKibanaPayload({ app, request, includeUserProvidedConfig, injectedVarsOverrides }) { + async function getLegacyKibanaPayload({ app, request, includeUserProvidedConfig, injectedVarsOverrides }) { const uiSettings = request.getUiSettingsService(); const translations = await request.getUiTranslations(); @@ -141,19 +141,23 @@ export function uiRenderMixin(kbnServer, server, config) { try { const request = reply.request; const translations = await request.getUiTranslations(); + const basePath = config.get('server.basePath'); i18n.init(translations); return reply.view('ui_app', { - app, - kibanaPayload: await getKibanaPayload({ - app, - request, - includeUserProvidedConfig, - injectedVarsOverrides - }), - bundlePath: `${config.get('server.basePath')}/bundles`, + uiPublicUrl: `${basePath}/ui`, + bootstrapScriptUrl: `${basePath}/bundles/app/${app.getId()}/bootstrap.js`, i18n: (id, options) => i18n.translate(id, options), + + injectedMetadata: { + legacyMetadata: await getLegacyKibanaPayload({ + app, + request, + includeUserProvidedConfig, + injectedVarsOverrides + }), + }, }); } catch (err) { reply(err); diff --git a/src/ui/ui_render/views/chrome.jade b/src/ui/ui_render/views/chrome.jade index b449c101b99ed..050b7060ec7e6 100644 --- a/src/ui/ui_render/views/chrome.jade +++ b/src/ui/ui_render/views/chrome.jade @@ -16,63 +16,63 @@ html(lang='en') font-style: normal; font-weight: 300; src: local('Open Sans Light'), local('OpenSans-Light'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_300.woff2') format('woff2'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_300.woff') format('woff'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_300.ttf') format('truetype'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_300.svg#OpenSans') format('svg'); + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_300.woff2') format('woff2'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_300.woff') format('woff'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_300.ttf') format('truetype'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_300.svg#OpenSans') format('svg'); } @font-face { font-family: 'Open Sans'; font-style: normal; font-weight: 400; src: local('Open Sans'), local('OpenSans'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_regular.woff2') format('woff2'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_regular.woff') format('woff'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_regular.ttf') format('truetype'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_regular.svg#OpenSans') format('svg'); + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_regular.woff2') format('woff2'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_regular.woff') format('woff'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_regular.ttf') format('truetype'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_regular.svg#OpenSans') format('svg'); } @font-face { font-family: 'Open Sans'; font-style: normal; font-weight: 600; src: local('Open Sans SemiBold'), local('OpenSans-SemiBold'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_600.woff2') format('woff2'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_600.woff') format('woff'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_600.ttf') format('truetype'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_600.svg#OpenSans') format('svg'); + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_600.woff2') format('woff2'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_600.woff') format('woff'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_600.ttf') format('truetype'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_600.svg#OpenSans') format('svg'); } @font-face { font-family: 'Open Sans'; font-style: normal; font-weight: 700; src: local('Open Sans Bold'), local('OpenSans-Bold'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_700.woff2') format('woff2'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_700.woff') format('woff'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_700.ttf') format('truetype'), - url('#{kibanaPayload.basePath}/ui/fonts/open_sans/open_sans_v15_latin_700.svg#OpenSans') format('svg'); + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_700.woff2') format('woff2'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_700.woff') format('woff'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_700.ttf') format('truetype'), + url('#{uiPublicUrl}/fonts/open_sans/open_sans_v15_latin_700.svg#OpenSans') format('svg'); } //- Favicons (generated from http://realfavicongenerator.net/) link( - rel='apple-touch-icon' sizes='180x180' href='#{kibanaPayload.basePath}/ui/favicons/apple-touch-icon.png' + rel='apple-touch-icon' sizes='180x180' href='#{uiPublicUrl}/favicons/apple-touch-icon.png' ) link( - rel='icon' type='image/png' href='#{kibanaPayload.basePath}/ui/favicons/favicon-32x32.png' sizes='32x32' + rel='icon' type='image/png' href='#{uiPublicUrl}/favicons/favicon-32x32.png' sizes='32x32' ) link( - rel='icon' type='image/png' href='#{kibanaPayload.basePath}/ui/favicons/favicon-16x16.png' sizes='16x16' + rel='icon' type='image/png' href='#{uiPublicUrl}/favicons/favicon-16x16.png' sizes='16x16' ) link( - rel='manifest' href='#{kibanaPayload.basePath}/ui/favicons/manifest.json' + rel='manifest' href='#{uiPublicUrl}/favicons/manifest.json' ) link( - rel='mask-icon' href='#{kibanaPayload.basePath}/ui/favicons/safari-pinned-tab.svg' color='#e8488b' + rel='mask-icon' href='#{uiPublicUrl}/favicons/safari-pinned-tab.svg' color='#e8488b' ) link( - rel='shortcut icon' href='#{kibanaPayload.basePath}/ui/favicons/favicon.ico' + rel='shortcut icon' href='#{uiPublicUrl}/favicons/favicon.ico' ) meta( - name='msapplication-config' content='#{kibanaPayload.basePath}/ui/favicons/browserconfig.xml' + name='msapplication-config' content='#{uiPublicUrl}/favicons/browserconfig.xml' ) meta( name='theme-color' content='#ffffff' @@ -121,5 +121,5 @@ html(lang='en') style#themeCss body(kbn-chrome, id='#{appName}-body') - kbn-initial-state(data=JSON.stringify(kibanaPayload)) + kbn-injected-metadata(data=JSON.stringify(injectedMetadata)) block content diff --git a/src/ui/ui_render/views/ui_app.jade b/src/ui/ui_render/views/ui_app.jade index e50bd41b12856..7c8b7c2efa74a 100644 --- a/src/ui/ui_render/views/ui_app.jade +++ b/src/ui/ui_render/views/ui_app.jade @@ -110,4 +110,4 @@ block content .kibanaWelcomeText | #{i18n('UI-WELCOME_MESSAGE', { defaultMessage: 'Loading Kibana' })} - script(src='#{bundlePath}/app/#{app.getId()}/bootstrap.js') + script(src=bootstrapScriptUrl) From a7fd8fb1859fc11e8b017187cb99453151a66185 Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 11 Jul 2018 16:50:53 -0700 Subject: [PATCH 02/30] fix test that parsed uiApp response --- src/ui/__tests__/ui_exports_replace_injected_vars.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/__tests__/ui_exports_replace_injected_vars.js b/src/ui/__tests__/ui_exports_replace_injected_vars.js index b6df18aef8b69..27c0ed4454140 100644 --- a/src/ui/__tests__/ui_exports_replace_injected_vars.js +++ b/src/ui/__tests__/ui_exports_replace_injected_vars.js @@ -29,7 +29,7 @@ import KbnServer from '../../server/kbn_server'; const getInjectedVarsFromResponse = (resp) => { const $ = cheerio.load(resp.payload); - const data = $('kbn-initial-state').attr('data'); + const data = $('kbn-injected-metadata').attr('data'); return JSON.parse(data).vars; }; From bb732528d3609ec40457a09333a6ff19c1b9cfb2 Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 11 Jul 2018 16:57:16 -0700 Subject: [PATCH 03/30] fix missed rename --- .../injected_metadata/injected_metadata_service.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/public/injected_metadata/injected_metadata_service.ts b/src/core/public/injected_metadata/injected_metadata_service.ts index fcc38b896dce7..8beabd173138f 100644 --- a/src/core/public/injected_metadata/injected_metadata_service.ts +++ b/src/core/public/injected_metadata/injected_metadata_service.ts @@ -20,17 +20,17 @@ import { deepFreeze } from './deep_freeze'; import { readInjectedMetadataFromDom } from './read_injected_metadata_from_dom'; -export interface InjectedState { +export interface InjectedMetadata { legacyMetadata: { [key: string]: any; }; } -export class InjectedStateService { - private state: InjectedState; +export class InjectedMetadataService { + private state: InjectedMetadata; - constructor(injectedStateForTesting?: InjectedState) { - this.state = deepFreeze(injectedStateForTesting || readInjectedMetadataFromDom()); + constructor(injectedMetadataForTesting?: InjectedMetadata) { + this.state = deepFreeze(injectedMetadataForTesting || readInjectedMetadataFromDom()); } public getLegacyMetadata() { From 0a8dcec1d10efbdfdd1b785b2a501e1152175d66 Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 11 Jul 2018 17:12:47 -0700 Subject: [PATCH 04/30] use a legacy platform service to bootstrap within the start() lifecycle --- src/core/public/core_system.ts | 21 +++++----- .../injected_metadata_service.ts | 16 +++++--- src/core/public/legacy_platform/index.ts | 20 ++++++++++ .../legacy_platform_service.ts | 39 +++++++++++++++++++ .../tests_bundle/tests_entry_template.js | 11 +++--- .../ui_bundles/new_platform_entry_template.js | 12 +++--- 6 files changed, 92 insertions(+), 27 deletions(-) create mode 100644 src/core/public/legacy_platform/index.ts create mode 100644 src/core/public/legacy_platform/legacy_platform_service.ts diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 3f196a3b1ea5e..611f1f06cf4c9 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -23,28 +23,27 @@ import 'custom-event-polyfill'; import 'whatwg-fetch'; import { InjectedMetadata, InjectedMetadataService } from './injected_metadata'; +import { LegacyPlatformService } from './legacy_platform'; interface CoreSystemParams { injectedMetadataForTesting?: InjectedMetadata; + bootstrapLegacyPlatform: () => void; } export class CoreSystem { private injectedMetadata: InjectedMetadataService; + private legacyPlatform: LegacyPlatformService; - constructor(params: CoreSystemParams = {}) { - const { injectedMetadataForTesting } = params; + constructor(params: CoreSystemParams) { + const { injectedMetadataForTesting, bootstrapLegacyPlatform } = params; this.injectedMetadata = new InjectedMetadataService(injectedMetadataForTesting); + this.legacyPlatform = new LegacyPlatformService(bootstrapLegacyPlatform); } - public initLegacyPlatform(bootstrapFn: () => void) { - /** - * Injects parts of the new platform into parts of the legacy platform - * so that legacy APIs/modules can mimic their new platform counterparts - */ - require('ui/metadata').__newPlatformInit__(this.injectedMetadata.getLegacyMetadata()); - - // call the legacy platform bootstrap function (bootstraps ui/chrome in apps and ui/test_harness in browser tests) - bootstrapFn(); + public start() { + this.legacyPlatform.start({ + injectedMetadata: this.injectedMetadata.start(), + }); } } diff --git a/src/core/public/injected_metadata/injected_metadata_service.ts b/src/core/public/injected_metadata/injected_metadata_service.ts index 8beabd173138f..62b34ef59ee25 100644 --- a/src/core/public/injected_metadata/injected_metadata_service.ts +++ b/src/core/public/injected_metadata/injected_metadata_service.ts @@ -27,13 +27,17 @@ export interface InjectedMetadata { } export class InjectedMetadataService { - private state: InjectedMetadata; + constructor(private injectedMetadataForTesting?: InjectedMetadata) {} - constructor(injectedMetadataForTesting?: InjectedMetadata) { - this.state = deepFreeze(injectedMetadataForTesting || readInjectedMetadataFromDom()); - } + public start() { + const state = deepFreeze( + this.injectedMetadataForTesting || (readInjectedMetadataFromDom() as InjectedMetadata) + ); - public getLegacyMetadata() { - return this.state.legacyMetadata; + return { + getLegacyMetadata() { + return state.legacyMetadata; + }, + }; } } diff --git a/src/core/public/legacy_platform/index.ts b/src/core/public/legacy_platform/index.ts new file mode 100644 index 0000000000000..b6477e9ba6f2d --- /dev/null +++ b/src/core/public/legacy_platform/index.ts @@ -0,0 +1,20 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export { LegacyPlatformService } from './legacy_platform_service'; diff --git a/src/core/public/legacy_platform/legacy_platform_service.ts b/src/core/public/legacy_platform/legacy_platform_service.ts new file mode 100644 index 0000000000000..3d2041b381ac6 --- /dev/null +++ b/src/core/public/legacy_platform/legacy_platform_service.ts @@ -0,0 +1,39 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { InjectedMetadataService } from '../injected_metadata'; + +interface Deps { + injectedMetadata: ReturnType; +} + +export class LegacyPlatformService { + constructor(private bootstrapLegacyPlatform: () => void) {} + + public start({ injectedMetadata }: Deps) { + /** + * Injects parts of the new platform into parts of the legacy platform + * so that legacy APIs/modules can mimic their new platform counterparts + */ + require('ui/metadata').__newPlatformInit__(injectedMetadata.getLegacyMetadata()); + + // call the legacy platform bootstrap function (bootstraps ui/chrome in apps and ui/test_harness in browser tests) + this.bootstrapLegacyPlatform(); + } +} diff --git a/src/core_plugins/tests_bundle/tests_entry_template.js b/src/core_plugins/tests_bundle/tests_entry_template.js index 31efa5b626afc..8c753dedcd9e4 100644 --- a/src/core_plugins/tests_bundle/tests_entry_template.js +++ b/src/core_plugins/tests_bundle/tests_entry_template.js @@ -69,10 +69,11 @@ const legacyMetadata = { new CoreSystem({ injectedMetadataForTesting: { legacyMetadata + }, + bootstrapLegacyPlatform: () => { + require('ui/test_harness'); + ${bundle.getRequires().join('\n ')} + require('ui/test_harness').bootstrap(); } -}).initLegacyPlatform(() => { - require('ui/test_harness'); - ${bundle.getRequires().join('\n ')} - require('ui/test_harness').bootstrap(); -}) +}).start() `; diff --git a/src/ui/ui_bundles/new_platform_entry_template.js b/src/ui/ui_bundles/new_platform_entry_template.js index e7ef5c8d02bdc..1333d7af8fa30 100644 --- a/src/ui/ui_bundles/new_platform_entry_template.js +++ b/src/ui/ui_bundles/new_platform_entry_template.js @@ -28,9 +28,11 @@ export const newPlatformEntryTemplate = (bundle) => ` import { CoreSystem } from '__kibanaCore__' -new CoreSystem().initLegacyPlatform(() => { - require('ui/chrome') - ${bundle.getRequires().join('\n ')} - require('ui/chrome').bootstrap(); -}) +new CoreSystem({ + bootstrapLegacyPlatform: () => { + require('ui/chrome') + ${bundle.getRequires().join('\n ')} + require('ui/chrome').bootstrap(); + } +}).start() `; From f1a698efeae3a6172b00599417507f9881068c8d Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 11 Jul 2018 17:25:00 -0700 Subject: [PATCH 05/30] [tests/ui_exports_replace_injected_vars] read vars from legacyMetadata --- src/ui/__tests__/ui_exports_replace_injected_vars.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/__tests__/ui_exports_replace_injected_vars.js b/src/ui/__tests__/ui_exports_replace_injected_vars.js index 27c0ed4454140..b7762ef104b90 100644 --- a/src/ui/__tests__/ui_exports_replace_injected_vars.js +++ b/src/ui/__tests__/ui_exports_replace_injected_vars.js @@ -30,7 +30,7 @@ import KbnServer from '../../server/kbn_server'; const getInjectedVarsFromResponse = (resp) => { const $ = cheerio.load(resp.payload); const data = $('kbn-injected-metadata').attr('data'); - return JSON.parse(data).vars; + return JSON.parse(data).legacyMetadata.vars; }; const injectReplacer = (kbnServer, replacer) => { From 9984fa5357b2e4dd403d82ba378e5c0e7156418e Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 12 Jul 2018 11:38:02 -0700 Subject: [PATCH 06/30] [core/injectedMetadata] use less permissive array type to avoid freezing strings --- src/core/public/injected_metadata/deep_freeze.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/public/injected_metadata/deep_freeze.ts b/src/core/public/injected_metadata/deep_freeze.ts index 45bdd93117c5d..33948fccef950 100644 --- a/src/core/public/injected_metadata/deep_freeze.ts +++ b/src/core/public/injected_metadata/deep_freeze.ts @@ -17,7 +17,7 @@ * under the License. */ -type Freezable = { [k: string]: any } | ArrayLike; +type Freezable = { [k: string]: any } | any[]; type RecursiveReadOnly = T extends Freezable ? Readonly<{ [K in keyof T]: RecursiveReadOnly }> From 6624b58d0560ae19c2a2dd16156acc0a79ca2d14 Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 12 Jul 2018 11:56:22 -0700 Subject: [PATCH 07/30] [ui/metadata/tests] remove unnecessary test --- src/ui/public/__tests__/metadata.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/ui/public/__tests__/metadata.js b/src/ui/public/__tests__/metadata.js index 13418d3e8b8c9..9cb5841f0d409 100644 --- a/src/ui/public/__tests__/metadata.js +++ b/src/ui/public/__tests__/metadata.js @@ -20,13 +20,6 @@ import expect from 'expect.js'; import { metadata } from '../metadata'; describe('ui/metadata', () => { - - - it('is same data as window.__KBN__', () => { - expect(metadata.version).to.equal(window.__KBN__.version); - expect(metadata.vars.kbnIndex).to.equal(window.__KBN__.vars.kbnIndex); - }); - it('is immutable', () => { expect(() => metadata.foo = 'something').to.throw; expect(() => metadata.version = 'something').to.throw; From 4f8fe21be748534472213052c2ca46125abdf63b Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 12 Jul 2018 14:54:37 -0700 Subject: [PATCH 08/30] implement feedback from Court --- src/core/public/core_system.ts | 25 ++++++------ .../injected_metadata_service.ts | 7 +--- .../read_injected_metadata_from_dom.ts | 33 ---------------- .../legacy_platform_service.ts | 38 +++++++++++++++---- .../tests_bundle/tests_entry_template.js | 13 +++++-- src/ui/public/chrome/chrome.js | 14 ++++--- src/ui/public/test_harness/test_harness.js | 8 ++-- ...ntry_template.js => app_entry_template.js} | 13 +++++-- src/ui/ui_render/views/chrome.jade | 5 +-- 9 files changed, 77 insertions(+), 79 deletions(-) delete mode 100644 src/core/public/injected_metadata/read_injected_metadata_from_dom.ts rename src/ui/ui_bundles/{new_platform_entry_template.js => app_entry_template.js} (75%) diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 611f1f06cf4c9..9dc4212258d3b 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -17,28 +17,29 @@ * under the License. */ -// import global polyfills before everything else -import 'babel-polyfill'; -import 'custom-event-polyfill'; -import 'whatwg-fetch'; - import { InjectedMetadata, InjectedMetadataService } from './injected_metadata'; import { LegacyPlatformService } from './legacy_platform'; -interface CoreSystemParams { - injectedMetadataForTesting?: InjectedMetadata; - bootstrapLegacyPlatform: () => void; +interface Params { + rootDomElement: HTMLElement; + injectedMetadata: InjectedMetadata; + requireLegacyFiles: () => void; + useLegacyTestHarness?: boolean; } export class CoreSystem { private injectedMetadata: InjectedMetadataService; private legacyPlatform: LegacyPlatformService; - constructor(params: CoreSystemParams) { - const { injectedMetadataForTesting, bootstrapLegacyPlatform } = params; + constructor(params: Params) { + const { rootDomElement, injectedMetadata, requireLegacyFiles, useLegacyTestHarness } = params; - this.injectedMetadata = new InjectedMetadataService(injectedMetadataForTesting); - this.legacyPlatform = new LegacyPlatformService(bootstrapLegacyPlatform); + this.injectedMetadata = new InjectedMetadataService(injectedMetadata); + this.legacyPlatform = new LegacyPlatformService( + rootDomElement, + requireLegacyFiles, + useLegacyTestHarness + ); } public start() { diff --git a/src/core/public/injected_metadata/injected_metadata_service.ts b/src/core/public/injected_metadata/injected_metadata_service.ts index 62b34ef59ee25..b1e928f335f66 100644 --- a/src/core/public/injected_metadata/injected_metadata_service.ts +++ b/src/core/public/injected_metadata/injected_metadata_service.ts @@ -18,7 +18,6 @@ */ import { deepFreeze } from './deep_freeze'; -import { readInjectedMetadataFromDom } from './read_injected_metadata_from_dom'; export interface InjectedMetadata { legacyMetadata: { @@ -27,12 +26,10 @@ export interface InjectedMetadata { } export class InjectedMetadataService { - constructor(private injectedMetadataForTesting?: InjectedMetadata) {} + constructor(private injectedMetadata: InjectedMetadata) {} public start() { - const state = deepFreeze( - this.injectedMetadataForTesting || (readInjectedMetadataFromDom() as InjectedMetadata) - ); + const state = deepFreeze(this.injectedMetadata); return { getLegacyMetadata() { diff --git a/src/core/public/injected_metadata/read_injected_metadata_from_dom.ts b/src/core/public/injected_metadata/read_injected_metadata_from_dom.ts deleted file mode 100644 index a903777ba507c..0000000000000 --- a/src/core/public/injected_metadata/read_injected_metadata_from_dom.ts +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -export function readInjectedMetadataFromDom() { - const el = document.querySelector('kbn-injected-metadata'); - - if (!el) { - throw new Error('unable to find element'); - } - - const json = el.getAttribute('data'); - if (!json) { - throw new Error(' does not have a data atttibute'); - } - - return JSON.parse(json); -} diff --git a/src/core/public/legacy_platform/legacy_platform_service.ts b/src/core/public/legacy_platform/legacy_platform_service.ts index 3d2041b381ac6..204f80524bb17 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.ts @@ -24,16 +24,40 @@ interface Deps { } export class LegacyPlatformService { - constructor(private bootstrapLegacyPlatform: () => void) {} + constructor( + private rootDomElement: HTMLElement, + private requireLegacyFiles: () => void, + private useLegacyTestHarness: boolean = false + ) {} public start({ injectedMetadata }: Deps) { - /** - * Injects parts of the new platform into parts of the legacy platform - * so that legacy APIs/modules can mimic their new platform counterparts - */ + // Inject parts of the new platform into parts of the legacy platform + // so that legacy APIs/modules can mimic their new platform counterparts require('ui/metadata').__newPlatformInit__(injectedMetadata.getLegacyMetadata()); - // call the legacy platform bootstrap function (bootstraps ui/chrome in apps and ui/test_harness in browser tests) - this.bootstrapLegacyPlatform(); + // Load the bootstrap module before loading the legacy platform files so that + // the bootstrap module can modify the environment a bit first + const bootstrapModule = this.loadBootstrapModule(); + + // require the files that will tie into the legacy platform + this.requireLegacyFiles(); + + bootstrapModule.bootstrap(this.rootDomElement); + } + + private loadBootstrapModule(): { + bootstrap: (rootDomElement: HTMLElement) => void; + } { + if (this.useLegacyTestHarness) { + // wrapped in NODE_ENV check so the `ui/test_harness` module + // is not included in the distributable + if (process.env.NODE_ENV !== 'production') { + return require('ui/test_harness'); + } + + throw new Error('tests bundle is not available in the distributable'); + } + + return require('ui/chrome'); } } diff --git a/src/core_plugins/tests_bundle/tests_entry_template.js b/src/core_plugins/tests_bundle/tests_entry_template.js index 8c753dedcd9e4..4720ac50a6078 100644 --- a/src/core_plugins/tests_bundle/tests_entry_template.js +++ b/src/core_plugins/tests_bundle/tests_entry_template.js @@ -29,6 +29,11 @@ export const createTestEntryTemplate = (defaultUiSettings) => (bundle) => ` * */ +// import global polyfills before everything else +import 'babel-polyfill'; +import 'custom-event-polyfill'; +import 'whatwg-fetch'; + import { CoreSystem } from '__kibanaCore__' const legacyMetadata = { @@ -67,13 +72,13 @@ const legacyMetadata = { }; new CoreSystem({ - injectedMetadataForTesting: { + injectedMetadata: { legacyMetadata }, - bootstrapLegacyPlatform: () => { - require('ui/test_harness'); + rootDomElement: document.body, + useLegacyTestHarness: true, + requireLegacyFiles: () => { ${bundle.getRequires().join('\n ')} - require('ui/test_harness').bootstrap(); } }).start() `; diff --git a/src/ui/public/chrome/chrome.js b/src/ui/public/chrome/chrome.js index 3a7e323553fc6..741d1eb629b62 100644 --- a/src/ui/public/chrome/chrome.js +++ b/src/ui/public/chrome/chrome.js @@ -72,7 +72,7 @@ themeApi(chrome, internals); translationsApi(chrome, internals); const waitForBootstrap = new Promise(resolve => { - chrome.bootstrap = function () { + chrome.bootstrap = function (targetDomElement) { // import chrome nav controls and hacks now so that they are executed after // everything else, can safely import the chrome, and interact with services // and such setup by all other modules @@ -80,8 +80,10 @@ const waitForBootstrap = new Promise(resolve => { require('uiExports/hacks'); chrome.setupAngular(); - angular.bootstrap(document.body, ['kibana']); - resolve(); + targetDomElement.setAttribute('id', 'kibana-body'); + targetDomElement.setAttribute('kbn-chrome', 'true'); + angular.bootstrap(targetDomElement, ['kibana']); + resolve(targetDomElement); }; }); @@ -100,10 +102,10 @@ const waitForBootstrap = new Promise(resolve => { * tests. Look into 'src/test_utils/public/stub_get_active_injector' for more information. */ chrome.dangerouslyGetActiveInjector = () => { - return waitForBootstrap.then(() => { - const $injector = angular.element(document.body).injector(); + return waitForBootstrap.then((targetDomElement) => { + const $injector = angular.element(targetDomElement).injector(); if (!$injector) { - return Promise.reject('document.body had no angular context after bootstrapping'); + return Promise.reject('targetDomElement had no angular context after bootstrapping'); } return $injector; }); diff --git a/src/ui/public/test_harness/test_harness.js b/src/ui/public/test_harness/test_harness.js index 73acc1aa303ed..3b5144145de69 100644 --- a/src/ui/public/test_harness/test_harness.js +++ b/src/ui/public/test_harness/test_harness.js @@ -41,9 +41,6 @@ if (query && query.mocha) { setupTestSharding(); -// allows test_harness.less to have higher priority selectors -document.body.setAttribute('id', 'test-harness-body'); - before(() => { // prevent accidental ajax requests sinon.useFakeXMLHttpRequest(); @@ -84,7 +81,10 @@ afterEach(function () { }); // Kick off mocha, called at the end of test entry files -export function bootstrap() { +export function bootstrap(targetDomElement) { + // allows test_harness.less to have higher priority selectors + targetDomElement.setAttribute('id', 'test-harness-body'); + // load the hacks since we aren't actually bootstrapping the // chrome, which is where the hacks would normally be loaded require('uiExports/hacks'); diff --git a/src/ui/ui_bundles/new_platform_entry_template.js b/src/ui/ui_bundles/app_entry_template.js similarity index 75% rename from src/ui/ui_bundles/new_platform_entry_template.js rename to src/ui/ui_bundles/app_entry_template.js index 1333d7af8fa30..b947130eb63e8 100644 --- a/src/ui/ui_bundles/new_platform_entry_template.js +++ b/src/ui/ui_bundles/app_entry_template.js @@ -17,7 +17,7 @@ * under the License. */ -export const newPlatformEntryTemplate = (bundle) => ` +export const appEntryTemplate = (bundle) => ` /** * Kibana entry file * @@ -26,13 +26,18 @@ export const newPlatformEntryTemplate = (bundle) => ` * context: ${bundle.getContext()} */ +// import global polyfills before everything else +import 'babel-polyfill'; +import 'custom-event-polyfill'; +import 'whatwg-fetch'; + import { CoreSystem } from '__kibanaCore__' new CoreSystem({ - bootstrapLegacyPlatform: () => { - require('ui/chrome') + injectedMetadata: JSON.parse(document.querySelector('kbn-injected-metadata').getAttribute('data')), + rootDomElement: document.body, + requireLegacyFiles: () => { ${bundle.getRequires().join('\n ')} - require('ui/chrome').bootstrap(); } }).start() `; diff --git a/src/ui/ui_render/views/chrome.jade b/src/ui/ui_render/views/chrome.jade index 050b7060ec7e6..28a4a680947ed 100644 --- a/src/ui/ui_render/views/chrome.jade +++ b/src/ui/ui_render/views/chrome.jade @@ -1,6 +1,3 @@ -- - var appName = 'kibana'; - block vars doctype html @@ -120,6 +117,6 @@ html(lang='en') //- good because we may use them to override EUI styles. style#themeCss - body(kbn-chrome, id='#{appName}-body') + body kbn-injected-metadata(data=JSON.stringify(injectedMetadata)) block content From 535711008fe3daf39b0ba2e70d092c4b86e9bb8d Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 12 Jul 2018 15:00:01 -0700 Subject: [PATCH 09/30] restore app_entry_template tests --- .../__tests__/app_entry_template.js | 49 +++++++++++++++++++ src/ui/ui_bundles/ui_bundles_controller.js | 4 +- 2 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 src/ui/ui_bundles/__tests__/app_entry_template.js diff --git a/src/ui/ui_bundles/__tests__/app_entry_template.js b/src/ui/ui_bundles/__tests__/app_entry_template.js new file mode 100644 index 0000000000000..79b57955ebd60 --- /dev/null +++ b/src/ui/ui_bundles/__tests__/app_entry_template.js @@ -0,0 +1,49 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import sinon from 'sinon'; +import expect from 'expect.js'; + +import { appEntryTemplate } from '../app_entry_template'; + +function createMockBundle() { + return { + getContext: sinon.stub().returns(''), + getRequires: sinon.stub().returns([]) + }; +} + +describe('ui bundles / appEntryTemplate', () => { + it('embeds bundle.getContext() result', () => { + const bundle = createMockBundle(); + bundle.getContext.returns('foo bar baz'); + expect(appEntryTemplate(bundle)).to.contain('foo bar baz'); + }); + + it('joins requires into list', () => { + const bundle = createMockBundle(); + const requires = [ + 'foo', + 'bar', + 'baz' + ]; + bundle.getRequires.returns(requires); + expect(appEntryTemplate(bundle)).to.contain(requires.join('\n ')); + }); +}); diff --git a/src/ui/ui_bundles/ui_bundles_controller.js b/src/ui/ui_bundles/ui_bundles_controller.js index 0a890393ee78c..887e5f0288993 100644 --- a/src/ui/ui_bundles/ui_bundles_controller.js +++ b/src/ui/ui_bundles/ui_bundles_controller.js @@ -27,7 +27,7 @@ import { makeRe } from 'minimatch'; import mkdirp from 'mkdirp'; import { UiBundle } from './ui_bundle'; -import { newPlatformEntryTemplate } from './new_platform_entry_template'; +import { appEntryTemplate } from './app_entry_template'; const mkdirpAsync = promisify(mkdirp); @@ -84,7 +84,7 @@ export class UiBundlesController { this.add({ id: uiApp.getId(), modules: [uiApp.getMainModuleId()], - template: newPlatformEntryTemplate, + template: appEntryTemplate, }); } } From b662c4de0a7954c89b3b8f764d8885869585a6a3 Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 12 Jul 2018 17:38:40 -0700 Subject: [PATCH 10/30] [ui/notify/tests] remove reference to window.__KBN__ --- src/ui/public/notify/__tests__/notifier.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ui/public/notify/__tests__/notifier.js b/src/ui/public/notify/__tests__/notifier.js index d05e48116bc94..da31599d3f63f 100644 --- a/src/ui/public/notify/__tests__/notifier.js +++ b/src/ui/public/notify/__tests__/notifier.js @@ -22,13 +22,12 @@ import ngMock from 'ng_mock'; import expect from 'expect.js'; import sinon from 'sinon'; import { Notifier } from '..'; +import { metadata } from 'ui/metadata'; describe('Notifier', function () { let $interval; let notifier; let params; - const version = window.__KBN__.version; - const buildNum = window.__KBN__.buildNum; const message = 'Oh, the humanity!'; const customText = 'fooMarkup'; const customParams = { @@ -334,13 +333,13 @@ describe('Notifier', function () { describe('when version is configured', function () { it('adds version to notification', function () { const notification = notify(fnName); - expect(notification.info.version).to.equal(version); + expect(notification.info.version).to.equal(metadata.version); }); }); describe('when build number is configured', function () { it('adds buildNum to notification', function () { const notification = notify(fnName); - expect(notification.info.buildNum).to.equal(buildNum); + expect(notification.info.buildNum).to.equal(metadata.buildNum); }); }); } From e404fa4bc39bd6a9f35b740838279635612eb47b Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 13 Jul 2018 07:30:24 -0700 Subject: [PATCH 11/30] [core] use *StartContract naming convention --- src/core/public/injected_metadata/index.ts | 6 +++++- .../public/injected_metadata/injected_metadata_service.ts | 2 ++ src/core/public/legacy_platform/legacy_platform_service.ts | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/core/public/injected_metadata/index.ts b/src/core/public/injected_metadata/index.ts index 662fb7f3033a8..5b23c2672d2e5 100644 --- a/src/core/public/injected_metadata/index.ts +++ b/src/core/public/injected_metadata/index.ts @@ -17,4 +17,8 @@ * under the License. */ -export { InjectedMetadata, InjectedMetadataService } from './injected_metadata_service'; +export { + InjectedMetadata, + InjectedMetadataService, + InjectedMetadataStartContract, +} from './injected_metadata_service'; diff --git a/src/core/public/injected_metadata/injected_metadata_service.ts b/src/core/public/injected_metadata/injected_metadata_service.ts index b1e928f335f66..dd73fab3c78a5 100644 --- a/src/core/public/injected_metadata/injected_metadata_service.ts +++ b/src/core/public/injected_metadata/injected_metadata_service.ts @@ -38,3 +38,5 @@ export class InjectedMetadataService { }; } } + +export type InjectedMetadataStartContract = ReturnType; diff --git a/src/core/public/legacy_platform/legacy_platform_service.ts b/src/core/public/legacy_platform/legacy_platform_service.ts index 204f80524bb17..d5b88f07d076b 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.ts @@ -17,10 +17,10 @@ * under the License. */ -import { InjectedMetadataService } from '../injected_metadata'; +import { InjectedMetadataStartContract } from '../injected_metadata'; interface Deps { - injectedMetadata: ReturnType; + injectedMetadata: InjectedMetadataStartContract; } export class LegacyPlatformService { From 2151adf8589b435596d788f6a6f8d0f1aeac1be9 Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 13 Jul 2018 14:17:03 -0700 Subject: [PATCH 12/30] [tslint] ignore fixtures in precommit hook --- src/dev/file.ts | 6 +++++- src/dev/tslint/pick_files_to_lint.ts | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/dev/file.ts b/src/dev/file.ts index b55e97ef34f4c..2e576a02ed791 100644 --- a/src/dev/file.ts +++ b/src/dev/file.ts @@ -17,7 +17,7 @@ * under the License. */ -import { dirname, extname, join, relative, resolve } from 'path'; +import { dirname, extname, join, relative, resolve, sep } from 'path'; import { REPO_ROOT } from './constants'; @@ -48,6 +48,10 @@ export class File { return this.ext === '.ts' || this.ext === '.tsx'; } + public isFixture() { + return this.relativePath.split(sep).includes('__fixtures__'); + } + public getRelativeParentDirs() { const parents: string[] = []; diff --git a/src/dev/tslint/pick_files_to_lint.ts b/src/dev/tslint/pick_files_to_lint.ts index 49f77f48230ee..7dc27c6a4d130 100644 --- a/src/dev/tslint/pick_files_to_lint.ts +++ b/src/dev/tslint/pick_files_to_lint.ts @@ -22,5 +22,5 @@ import { ToolingLog } from '@kbn/dev-utils'; import { File } from '../file'; export function pickFilesToLint(log: ToolingLog, files: File[]) { - return files.filter(file => file.isTypescript()); + return files.filter(file => file.isTypescript() && !file.isFixture()); } From 04af61a3459988c447f910ea61af7cb0fcd613c6 Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 13 Jul 2018 14:50:30 -0700 Subject: [PATCH 13/30] [core/public] add tests, use *Params in constructors --- src/core/public/core_system.test.ts | 120 ++++++++++++++++ src/core/public/core_system.ts | 22 ++- .../__fixtures__/frozen_object_mutation.ts | 33 +++++ .../frozen_object_mutation.tsconfig.json | 13 ++ .../injected_metadata/deep_freeze.test.ts | 86 ++++++++++++ src/core/public/injected_metadata/index.ts | 2 +- .../injected_metadata_service.test.ts | 64 +++++++++ .../injected_metadata_service.ts | 12 +- src/core/public/legacy_platform/index.ts | 2 +- .../legacy_platform_service.test.ts | 129 ++++++++++++++++++ .../legacy_platform_service.ts | 18 +-- tsconfig.json | 3 + 12 files changed, 477 insertions(+), 27 deletions(-) create mode 100644 src/core/public/core_system.test.ts create mode 100644 src/core/public/injected_metadata/__fixtures__/frozen_object_mutation.ts create mode 100644 src/core/public/injected_metadata/__fixtures__/frozen_object_mutation.tsconfig.json create mode 100644 src/core/public/injected_metadata/deep_freeze.test.ts create mode 100644 src/core/public/injected_metadata/injected_metadata_service.test.ts create mode 100644 src/core/public/legacy_platform/legacy_platform_service.test.ts diff --git a/src/core/public/core_system.test.ts b/src/core/public/core_system.test.ts new file mode 100644 index 0000000000000..ae3f5e3556006 --- /dev/null +++ b/src/core/public/core_system.test.ts @@ -0,0 +1,120 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +jest.mock('./legacy_platform', () => ({ + LegacyPlatformService: jest.fn(function MockInjectedMetadataService(this: any) { + this.start = jest.fn(); + }), +})); + +const mockInjectedMetadataStartContract = {}; +jest.mock('./injected_metadata', () => ({ + InjectedMetadataService: jest.fn(function MockInjectedMetadataService(this: any) { + this.start = jest.fn().mockReturnValue(mockInjectedMetadataStartContract); + }), +})); + +import { CoreSystem } from './core_system'; +import { InjectedMetadataService } from './injected_metadata'; +import { LegacyPlatformService } from './legacy_platform'; + +const defaultCoreSystemParams = { + rootDomElement: null!, + injectedMetadata: {} as any, + requireLegacyFiles: jest.fn(), +}; + +function getFirstStartMethodMock< + T extends typeof InjectedMetadataService | typeof LegacyPlatformService +>(Service: T): jest.MockInstance { + const mockService: jest.MockInstance = Service as any; + return mockService.mock.instances[0].start as any; +} + +beforeEach(() => { + jest.clearAllMocks(); +}); + +describe('constructor', () => { + it('creates instances of services', () => { + // tslint:disable no-unused-expression + new CoreSystem({ + ...defaultCoreSystemParams, + }); + + expect(InjectedMetadataService).toHaveBeenCalledTimes(1); + expect(LegacyPlatformService).toHaveBeenCalledTimes(1); + }); + + it('passes injectedMetadata param to InjectedMetadataService', () => { + const injectedMetadata = { injectedMetadata: true } as any; + + // tslint:disable no-unused-expression + new CoreSystem({ + ...defaultCoreSystemParams, + injectedMetadata, + }); + + expect(InjectedMetadataService).toHaveBeenCalledTimes(1); + expect(InjectedMetadataService).toHaveBeenCalledWith({ + injectedMetadata, + }); + }); + + it('passes rootDomElement, requireLegacyFiles, and useLegacyTestHarness to LegacyPlatformService', () => { + const rootDomElement = { rootDomElement: true } as any; + const requireLegacyFiles = { requireLegacyFiles: true } as any; + const useLegacyTestHarness = { useLegacyTestHarness: true } as any; + + // tslint:disable no-unused-expression + new CoreSystem({ + ...defaultCoreSystemParams, + rootDomElement, + requireLegacyFiles, + useLegacyTestHarness, + }); + + expect(LegacyPlatformService).toHaveBeenCalledTimes(1); + expect(LegacyPlatformService).toHaveBeenCalledWith({ + rootDomElement, + requireLegacyFiles, + useLegacyTestHarness, + }); + }); +}); + +describe('#start()', () => { + it('calls lifecycleSystem#start() and injectedMetadata#start()', () => { + const core = new CoreSystem({ + ...defaultCoreSystemParams, + }); + + expect(core.start()).toBe(undefined); + + const injectedMetadataStartMock = getFirstStartMethodMock(InjectedMetadataService); + expect(injectedMetadataStartMock).toHaveBeenCalledTimes(1); + expect(injectedMetadataStartMock).toHaveBeenCalledWith(); + + const legacyPlatformStartMock = getFirstStartMethodMock(LegacyPlatformService); + expect(legacyPlatformStartMock).toHaveBeenCalledTimes(1); + expect(legacyPlatformStartMock).toHaveBeenCalledWith({ + injectedMetadata: mockInjectedMetadataStartContract, + }); + }); +}); diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 9dc4212258d3b..22f75129f31fe 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -17,15 +17,10 @@ * under the License. */ -import { InjectedMetadata, InjectedMetadataService } from './injected_metadata'; -import { LegacyPlatformService } from './legacy_platform'; +import { InjectedMetadataService, InjectedMetadataServiceParams } from './injected_metadata'; +import { LegacyPlatformService, LegacyPlatformServiceParams } from './legacy_platform'; -interface Params { - rootDomElement: HTMLElement; - injectedMetadata: InjectedMetadata; - requireLegacyFiles: () => void; - useLegacyTestHarness?: boolean; -} +type Params = InjectedMetadataServiceParams & LegacyPlatformServiceParams; export class CoreSystem { private injectedMetadata: InjectedMetadataService; @@ -34,12 +29,15 @@ export class CoreSystem { constructor(params: Params) { const { rootDomElement, injectedMetadata, requireLegacyFiles, useLegacyTestHarness } = params; - this.injectedMetadata = new InjectedMetadataService(injectedMetadata); - this.legacyPlatform = new LegacyPlatformService( + this.injectedMetadata = new InjectedMetadataService({ + injectedMetadata, + }); + + this.legacyPlatform = new LegacyPlatformService({ rootDomElement, requireLegacyFiles, - useLegacyTestHarness - ); + useLegacyTestHarness, + }); } public start() { diff --git a/src/core/public/injected_metadata/__fixtures__/frozen_object_mutation.ts b/src/core/public/injected_metadata/__fixtures__/frozen_object_mutation.ts new file mode 100644 index 0000000000000..ab9fe4da923ce --- /dev/null +++ b/src/core/public/injected_metadata/__fixtures__/frozen_object_mutation.ts @@ -0,0 +1,33 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { deepFreeze } from '../deep_freeze'; + +const obj = deepFreeze({ + foo: { + bar: { + baz: 1, + }, + }, +}); + +delete obj.foo; +obj.foo = 1; +obj.foo.bar.baz = 2; +obj.foo.bar.box = false; diff --git a/src/core/public/injected_metadata/__fixtures__/frozen_object_mutation.tsconfig.json b/src/core/public/injected_metadata/__fixtures__/frozen_object_mutation.tsconfig.json new file mode 100644 index 0000000000000..aaedce798435d --- /dev/null +++ b/src/core/public/injected_metadata/__fixtures__/frozen_object_mutation.tsconfig.json @@ -0,0 +1,13 @@ +{ + "compilerOptions": { + "strict": true, + "skipLibCheck": true, + "lib": [ + "esnext" + ] + }, + "include": [ + "frozen_object_mutation.ts", + "../deep_freeze.ts" + ] +} diff --git a/src/core/public/injected_metadata/deep_freeze.test.ts b/src/core/public/injected_metadata/deep_freeze.test.ts new file mode 100644 index 0000000000000..7d93a97ad73de --- /dev/null +++ b/src/core/public/injected_metadata/deep_freeze.test.ts @@ -0,0 +1,86 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { resolve } from 'path'; + +import execa from 'execa'; + +import { deepFreeze } from './deep_freeze'; + +it('returns the first argument with all original references', () => { + const a = {}; + const b = {}; + const c = { a, b }; + + const frozen = deepFreeze(c); + expect(frozen).toBe(c); + expect(frozen.a).toBe(a); + expect(frozen.b).toBe(b); +}); + +it('prevents adding properties to argument', () => { + const frozen = deepFreeze({}); + expect(() => { + // @ts-ignore ts knows this shouldn't be possible, but just making sure + frozen.foo = true; + }).toThrowError(`object is not extensible`); +}); + +it('prevents changing properties on argument', () => { + const frozen = deepFreeze({ foo: false }); + expect(() => { + // @ts-ignore ts knows this shouldn't be possible, but just making sure + frozen.foo = true; + }).toThrowError(`read only property 'foo'`); +}); + +it('prevents changing properties on nested children of argument', () => { + const frozen = deepFreeze({ foo: { bar: { baz: { box: 1 } } } }); + expect(() => { + // @ts-ignore ts knows this shouldn't be possible, but just making sure + frozen.foo.bar.baz.box = 2; + }).toThrowError(`read only property 'box'`); +}); + +it('types return values to prevent mutations in typescript', async () => { + const result = await execa.stdout( + 'tsc', + [ + '--noEmit', + '--project', + resolve(__dirname, '__fixtures__/frozen_object_mutation.tsconfig.json'), + ], + { + cwd: resolve(__dirname, '__fixtures__'), + reject: false, + } + ); + + const errorCodeRe = /\serror\s(TS\d{4}):/g; + const errorCodes = []; + while (true) { + const match = errorCodeRe.exec(result); + if (!match) { + break; + } + errorCodes.push(match[1]); + } + + expect(errorCodes).toEqual(['TS2704', 'TS2540', 'TS2540', 'TS2339']); +}); diff --git a/src/core/public/injected_metadata/index.ts b/src/core/public/injected_metadata/index.ts index 5b23c2672d2e5..ae3135fe5523b 100644 --- a/src/core/public/injected_metadata/index.ts +++ b/src/core/public/injected_metadata/index.ts @@ -18,7 +18,7 @@ */ export { - InjectedMetadata, InjectedMetadataService, + InjectedMetadataServiceParams, InjectedMetadataStartContract, } from './injected_metadata_service'; diff --git a/src/core/public/injected_metadata/injected_metadata_service.test.ts b/src/core/public/injected_metadata/injected_metadata_service.test.ts new file mode 100644 index 0000000000000..ad1e74efcee9b --- /dev/null +++ b/src/core/public/injected_metadata/injected_metadata_service.test.ts @@ -0,0 +1,64 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { InjectedMetadataService } from './injected_metadata_service'; + +describe('#start()', () => { + it('returns the start contract', () => { + const injectedMetadata = new InjectedMetadataService({ + injectedMetadata: {} as any, + }); + + const contract = injectedMetadata.start(); + expect(contract).toEqual({ + getLegacyMetadata: expect.any(Function), + }); + }); + + it('deeply freezes its injectedMetadata param', () => { + const params = { + injectedMetadata: { foo: true } as any, + }; + + const injectedMetadata = new InjectedMetadataService(params); + + expect(() => { + params.injectedMetadata.foo = false; + }).not.toThrowError(); + + injectedMetadata.start(); + + expect(() => { + params.injectedMetadata.foo = true; + }).toThrowError(`read only property 'foo'`); + }); +}); + +describe('start.getLegacyMetadata()', () => { + it('returns injectedMetadata.legacyMetadata', () => { + const injectedMetadata = new InjectedMetadataService({ + injectedMetadata: { + legacyMetadata: 'foo', + } as any, + }); + + const contract = injectedMetadata.start(); + expect(contract.getLegacyMetadata()).toBe('foo'); + }); +}); diff --git a/src/core/public/injected_metadata/injected_metadata_service.ts b/src/core/public/injected_metadata/injected_metadata_service.ts index dd73fab3c78a5..710e0c567ebbf 100644 --- a/src/core/public/injected_metadata/injected_metadata_service.ts +++ b/src/core/public/injected_metadata/injected_metadata_service.ts @@ -19,17 +19,19 @@ import { deepFreeze } from './deep_freeze'; -export interface InjectedMetadata { - legacyMetadata: { - [key: string]: any; +export interface InjectedMetadataServiceParams { + injectedMetadata: { + legacyMetadata: { + [key: string]: any; + }; }; } export class InjectedMetadataService { - constructor(private injectedMetadata: InjectedMetadata) {} + constructor(private params: InjectedMetadataServiceParams) {} public start() { - const state = deepFreeze(this.injectedMetadata); + const state = deepFreeze(this.params.injectedMetadata); return { getLegacyMetadata() { diff --git a/src/core/public/legacy_platform/index.ts b/src/core/public/legacy_platform/index.ts index b6477e9ba6f2d..a5e3d698c8123 100644 --- a/src/core/public/legacy_platform/index.ts +++ b/src/core/public/legacy_platform/index.ts @@ -17,4 +17,4 @@ * under the License. */ -export { LegacyPlatformService } from './legacy_platform_service'; +export { LegacyPlatformService, LegacyPlatformServiceParams } from './legacy_platform_service'; diff --git a/src/core/public/legacy_platform/legacy_platform_service.test.ts b/src/core/public/legacy_platform/legacy_platform_service.test.ts new file mode 100644 index 0000000000000..cff1b6ced0e4c --- /dev/null +++ b/src/core/public/legacy_platform/legacy_platform_service.test.ts @@ -0,0 +1,129 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +const mockLoadOrder: string[] = []; + +jest.mock('ui/metadata', () => { + mockLoadOrder.push('ui/metadata'); + return { + __newPlatformInit__: jest.fn(), + }; +}); + +jest.mock('ui/chrome', () => { + mockLoadOrder.push('ui/chrome'); + return { + bootstrap: jest.fn(), + }; +}); + +jest.mock('ui/test_harness', () => { + mockLoadOrder.push('ui/test_harness'); + return { + bootstrap: jest.fn(), + }; +}); + +import { LegacyPlatformService } from './legacy_platform_service'; + +beforeEach(() => { + jest.clearAllMocks(); + injectedMetadataStartContract.getLegacyMetadata.mockReset(); + jest.resetModules(); + mockLoadOrder.length = 0; +}); + +const injectedMetadataStartContract = { + getLegacyMetadata: jest.fn(), +}; + +const requireLegacyFiles = jest.fn(() => { + mockLoadOrder.push('legacy files'); +}); + +describe('#start()', () => { + describe('default', () => { + it('does not return a start contract', () => { + const legacyPlatform = new LegacyPlatformService({ + rootDomElement: null!, + requireLegacyFiles, + }); + + const startContract = legacyPlatform.start({ + injectedMetadata: injectedMetadataStartContract, + }); + expect(startContract).toBe(undefined); + }); + + it('passes legacy metadata from injectedVars to ui/metadata', () => { + const legacyMetadata = { isLegacyMetadata: true }; + injectedMetadataStartContract.getLegacyMetadata.mockReturnValue(legacyMetadata); + + const legacyPlatform = new LegacyPlatformService({ + rootDomElement: null!, + requireLegacyFiles, + }); + + legacyPlatform.start({ + injectedMetadata: injectedMetadataStartContract, + }); + + const newPlatformInit = require('ui/metadata').__newPlatformInit__; + expect(newPlatformInit).toHaveBeenCalledTimes(1); + expect(newPlatformInit).toHaveBeenCalledWith(legacyMetadata); + }); + }); + + describe('load order', () => { + describe('useLegacyTestHarness = false', () => { + it('loads ui/modules before ui/chrome, and both before legacy files', () => { + const legacyPlatform = new LegacyPlatformService({ + rootDomElement: null!, + requireLegacyFiles, + }); + + expect(mockLoadOrder).toEqual([]); + + legacyPlatform.start({ + injectedMetadata: injectedMetadataStartContract, + }); + + expect(mockLoadOrder).toEqual(['ui/metadata', 'ui/chrome', 'legacy files']); + }); + }); + + describe('useLegacyTestHarness = true', () => { + it('loads ui/modules before ui/test_harness, and both before legacy files', () => { + const legacyPlatform = new LegacyPlatformService({ + rootDomElement: null!, + requireLegacyFiles, + useLegacyTestHarness: true, + }); + + expect(mockLoadOrder).toEqual([]); + + legacyPlatform.start({ + injectedMetadata: injectedMetadataStartContract, + }); + + expect(mockLoadOrder).toEqual(['ui/metadata', 'ui/test_harness', 'legacy files']); + }); + }); + }); +}); diff --git a/src/core/public/legacy_platform/legacy_platform_service.ts b/src/core/public/legacy_platform/legacy_platform_service.ts index d5b88f07d076b..ec314cc04e275 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.ts @@ -23,12 +23,14 @@ interface Deps { injectedMetadata: InjectedMetadataStartContract; } +export interface LegacyPlatformServiceParams { + rootDomElement: HTMLElement; + requireLegacyFiles: () => void; + useLegacyTestHarness?: boolean; +} + export class LegacyPlatformService { - constructor( - private rootDomElement: HTMLElement, - private requireLegacyFiles: () => void, - private useLegacyTestHarness: boolean = false - ) {} + constructor(private params: LegacyPlatformServiceParams) {} public start({ injectedMetadata }: Deps) { // Inject parts of the new platform into parts of the legacy platform @@ -40,15 +42,15 @@ export class LegacyPlatformService { const bootstrapModule = this.loadBootstrapModule(); // require the files that will tie into the legacy platform - this.requireLegacyFiles(); + this.params.requireLegacyFiles(); - bootstrapModule.bootstrap(this.rootDomElement); + bootstrapModule.bootstrap(this.params.rootDomElement); } private loadBootstrapModule(): { bootstrap: (rootDomElement: HTMLElement) => void; } { - if (this.useLegacyTestHarness) { + if (this.params.useLegacyTestHarness) { // wrapped in NODE_ENV check so the `ui/test_harness` module // is not included in the distributable if (process.env.NODE_ENV !== 'production') { diff --git a/tsconfig.json b/tsconfig.json index dea27f75835f1..d6492950fc0a1 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -43,5 +43,8 @@ }, "include": [ "src/**/*" + ], + "exclude": [ + "src/**/__fixtures__/**/*" ] } From 9c76bfb6dbd539038eee77f1a32456ca970cbd1f Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 13 Jul 2018 15:00:02 -0700 Subject: [PATCH 14/30] [core/public/tests] fix type definitions --- src/core/public/core_system.test.ts | 52 ++++++++++++++--------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/src/core/public/core_system.test.ts b/src/core/public/core_system.test.ts index ae3f5e3556006..8366700329cf6 100644 --- a/src/core/public/core_system.test.ts +++ b/src/core/public/core_system.test.ts @@ -17,22 +17,29 @@ * under the License. */ -jest.mock('./legacy_platform', () => ({ - LegacyPlatformService: jest.fn(function MockInjectedMetadataService(this: any) { +import { InjectedMetadataService } from './injected_metadata'; +import { LegacyPlatformService } from './legacy_platform'; + +const MockLegacyPlatformService = jest.fn( + function _MockLegacyPlatformService(this: any) { this.start = jest.fn(); - }), + } +); +jest.mock('./legacy_platform', () => ({ + LegacyPlatformService: MockLegacyPlatformService, })); const mockInjectedMetadataStartContract = {}; -jest.mock('./injected_metadata', () => ({ - InjectedMetadataService: jest.fn(function MockInjectedMetadataService(this: any) { +const MockInjectedMetadataService = jest.fn( + function _MockInjectedMetadataService(this: any) { this.start = jest.fn().mockReturnValue(mockInjectedMetadataStartContract); - }), + } +); +jest.mock('./injected_metadata', () => ({ + InjectedMetadataService: MockInjectedMetadataService, })); import { CoreSystem } from './core_system'; -import { InjectedMetadataService } from './injected_metadata'; -import { LegacyPlatformService } from './legacy_platform'; const defaultCoreSystemParams = { rootDomElement: null!, @@ -40,13 +47,6 @@ const defaultCoreSystemParams = { requireLegacyFiles: jest.fn(), }; -function getFirstStartMethodMock< - T extends typeof InjectedMetadataService | typeof LegacyPlatformService ->(Service: T): jest.MockInstance { - const mockService: jest.MockInstance = Service as any; - return mockService.mock.instances[0].start as any; -} - beforeEach(() => { jest.clearAllMocks(); }); @@ -58,8 +58,8 @@ describe('constructor', () => { ...defaultCoreSystemParams, }); - expect(InjectedMetadataService).toHaveBeenCalledTimes(1); - expect(LegacyPlatformService).toHaveBeenCalledTimes(1); + expect(MockInjectedMetadataService).toHaveBeenCalledTimes(1); + expect(MockLegacyPlatformService).toHaveBeenCalledTimes(1); }); it('passes injectedMetadata param to InjectedMetadataService', () => { @@ -71,8 +71,8 @@ describe('constructor', () => { injectedMetadata, }); - expect(InjectedMetadataService).toHaveBeenCalledTimes(1); - expect(InjectedMetadataService).toHaveBeenCalledWith({ + expect(MockInjectedMetadataService).toHaveBeenCalledTimes(1); + expect(MockInjectedMetadataService).toHaveBeenCalledWith({ injectedMetadata, }); }); @@ -90,8 +90,8 @@ describe('constructor', () => { useLegacyTestHarness, }); - expect(LegacyPlatformService).toHaveBeenCalledTimes(1); - expect(LegacyPlatformService).toHaveBeenCalledWith({ + expect(MockLegacyPlatformService).toHaveBeenCalledTimes(1); + expect(MockLegacyPlatformService).toHaveBeenCalledWith({ rootDomElement, requireLegacyFiles, useLegacyTestHarness, @@ -107,13 +107,11 @@ describe('#start()', () => { expect(core.start()).toBe(undefined); - const injectedMetadataStartMock = getFirstStartMethodMock(InjectedMetadataService); - expect(injectedMetadataStartMock).toHaveBeenCalledTimes(1); - expect(injectedMetadataStartMock).toHaveBeenCalledWith(); + expect(MockInjectedMetadataService.mock.instances[0].start).toHaveBeenCalledTimes(1); + expect(MockInjectedMetadataService.mock.instances[0].start).toHaveBeenCalledWith(); - const legacyPlatformStartMock = getFirstStartMethodMock(LegacyPlatformService); - expect(legacyPlatformStartMock).toHaveBeenCalledTimes(1); - expect(legacyPlatformStartMock).toHaveBeenCalledWith({ + expect(MockLegacyPlatformService.mock.instances[0].start).toHaveBeenCalledTimes(1); + expect(MockLegacyPlatformService.mock.instances[0].start).toHaveBeenCalledWith({ injectedMetadata: mockInjectedMetadataStartContract, }); }); From 30731597055beef830444d038e2f8b29e4b00b20 Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 13 Jul 2018 15:08:54 -0700 Subject: [PATCH 15/30] [core/public/legacyPlatform] test arguments to legacy bootstrap module --- .../legacy_platform_service.test.ts | 77 +++++++++++++------ 1 file changed, 55 insertions(+), 22 deletions(-) diff --git a/src/core/public/legacy_platform/legacy_platform_service.test.ts b/src/core/public/legacy_platform/legacy_platform_service.test.ts index cff1b6ced0e4c..3fc18913f6d92 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.test.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.test.ts @@ -19,29 +19,43 @@ const mockLoadOrder: string[] = []; +const mockUiMetadataInit = jest.fn(); jest.mock('ui/metadata', () => { mockLoadOrder.push('ui/metadata'); return { - __newPlatformInit__: jest.fn(), + __newPlatformInit__: mockUiMetadataInit, }; }); +const mockUiChromeBootstrap = jest.fn(); jest.mock('ui/chrome', () => { mockLoadOrder.push('ui/chrome'); return { - bootstrap: jest.fn(), + bootstrap: mockUiChromeBootstrap, }; }); +const mockUiTestHarnessBootstrap = jest.fn(); jest.mock('ui/test_harness', () => { mockLoadOrder.push('ui/test_harness'); return { - bootstrap: jest.fn(), + bootstrap: mockUiTestHarnessBootstrap, }; }); import { LegacyPlatformService } from './legacy_platform_service'; +const injectedMetadataStartContract = { + getLegacyMetadata: jest.fn(), +}; + +const defaultParams = { + rootDomElement: { someDomElement: true } as any, + requireLegacyFiles: jest.fn(() => { + mockLoadOrder.push('legacy files'); + }), +}; + beforeEach(() => { jest.clearAllMocks(); injectedMetadataStartContract.getLegacyMetadata.mockReset(); @@ -49,20 +63,11 @@ beforeEach(() => { mockLoadOrder.length = 0; }); -const injectedMetadataStartContract = { - getLegacyMetadata: jest.fn(), -}; - -const requireLegacyFiles = jest.fn(() => { - mockLoadOrder.push('legacy files'); -}); - describe('#start()', () => { describe('default', () => { it('does not return a start contract', () => { const legacyPlatform = new LegacyPlatformService({ - rootDomElement: null!, - requireLegacyFiles, + ...defaultParams, }); const startContract = legacyPlatform.start({ @@ -76,17 +81,47 @@ describe('#start()', () => { injectedMetadataStartContract.getLegacyMetadata.mockReturnValue(legacyMetadata); const legacyPlatform = new LegacyPlatformService({ - rootDomElement: null!, - requireLegacyFiles, + ...defaultParams, }); legacyPlatform.start({ injectedMetadata: injectedMetadataStartContract, }); - const newPlatformInit = require('ui/metadata').__newPlatformInit__; - expect(newPlatformInit).toHaveBeenCalledTimes(1); - expect(newPlatformInit).toHaveBeenCalledWith(legacyMetadata); + expect(mockUiMetadataInit).toHaveBeenCalledTimes(1); + expect(mockUiMetadataInit).toHaveBeenCalledWith(legacyMetadata); + }); + + describe('useLegacyTestHarness = false', () => { + it('passes the rootDomElement to ui/chrome', () => { + const legacyPlatform = new LegacyPlatformService({ + ...defaultParams, + }); + + legacyPlatform.start({ + injectedMetadata: injectedMetadataStartContract, + }); + + expect(mockUiTestHarnessBootstrap).not.toHaveBeenCalled(); + expect(mockUiChromeBootstrap).toHaveBeenCalledTimes(1); + expect(mockUiChromeBootstrap).toHaveBeenCalledWith(defaultParams.rootDomElement); + }); + }); + describe('useLegacyTestHarness = true', () => { + it('passes the rootDomElement to ui/test_harness', () => { + const legacyPlatform = new LegacyPlatformService({ + ...defaultParams, + useLegacyTestHarness: true, + }); + + legacyPlatform.start({ + injectedMetadata: injectedMetadataStartContract, + }); + + expect(mockUiChromeBootstrap).not.toHaveBeenCalled(); + expect(mockUiTestHarnessBootstrap).toHaveBeenCalledTimes(1); + expect(mockUiTestHarnessBootstrap).toHaveBeenCalledWith(defaultParams.rootDomElement); + }); }); }); @@ -94,8 +129,7 @@ describe('#start()', () => { describe('useLegacyTestHarness = false', () => { it('loads ui/modules before ui/chrome, and both before legacy files', () => { const legacyPlatform = new LegacyPlatformService({ - rootDomElement: null!, - requireLegacyFiles, + ...defaultParams, }); expect(mockLoadOrder).toEqual([]); @@ -111,8 +145,7 @@ describe('#start()', () => { describe('useLegacyTestHarness = true', () => { it('loads ui/modules before ui/test_harness, and both before legacy files', () => { const legacyPlatform = new LegacyPlatformService({ - rootDomElement: null!, - requireLegacyFiles, + ...defaultParams, useLegacyTestHarness: true, }); From 450e6b1a8528d596ca15a9506a5bfad0b7643a13 Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 13 Jul 2018 15:10:55 -0700 Subject: [PATCH 16/30] [core/public/core/tests] use correct services for mock generics --- src/core/public/core_system.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/public/core_system.test.ts b/src/core/public/core_system.test.ts index 8366700329cf6..fbdd6dc999247 100644 --- a/src/core/public/core_system.test.ts +++ b/src/core/public/core_system.test.ts @@ -20,7 +20,7 @@ import { InjectedMetadataService } from './injected_metadata'; import { LegacyPlatformService } from './legacy_platform'; -const MockLegacyPlatformService = jest.fn( +const MockLegacyPlatformService = jest.fn( function _MockLegacyPlatformService(this: any) { this.start = jest.fn(); } @@ -30,7 +30,7 @@ jest.mock('./legacy_platform', () => ({ })); const mockInjectedMetadataStartContract = {}; -const MockInjectedMetadataService = jest.fn( +const MockInjectedMetadataService = jest.fn( function _MockInjectedMetadataService(this: any) { this.start = jest.fn().mockReturnValue(mockInjectedMetadataStartContract); } From 675f0327b23c537cffe786cf25ac53257a592c36 Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 13 Jul 2018 15:33:09 -0700 Subject: [PATCH 17/30] [core/public] list explicit params for core service --- src/core/public/core_system.ts | 11 ++++++++--- src/core/public/injected_metadata/index.ts | 2 +- .../injected_metadata/injected_metadata_service.ts | 4 ++-- src/core/public/legacy_platform/index.ts | 2 +- .../public/legacy_platform/legacy_platform_service.ts | 4 ++-- 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 22f75129f31fe..4b0e0919e6ca2 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -17,10 +17,15 @@ * under the License. */ -import { InjectedMetadataService, InjectedMetadataServiceParams } from './injected_metadata'; -import { LegacyPlatformService, LegacyPlatformServiceParams } from './legacy_platform'; +import { InjectedMetadataParams, InjectedMetadataService } from './injected_metadata'; +import { LegacyPlatformParams, LegacyPlatformService } from './legacy_platform'; -type Params = InjectedMetadataServiceParams & LegacyPlatformServiceParams; +interface Params { + injectedMetadata: InjectedMetadataParams['injectedMetadata']; + rootDomElement: LegacyPlatformParams['rootDomElement']; + requireLegacyFiles: LegacyPlatformParams['requireLegacyFiles']; + useLegacyTestHarness?: LegacyPlatformParams['useLegacyTestHarness']; +} export class CoreSystem { private injectedMetadata: InjectedMetadataService; diff --git a/src/core/public/injected_metadata/index.ts b/src/core/public/injected_metadata/index.ts index ae3135fe5523b..e8979fc0c3774 100644 --- a/src/core/public/injected_metadata/index.ts +++ b/src/core/public/injected_metadata/index.ts @@ -19,6 +19,6 @@ export { InjectedMetadataService, - InjectedMetadataServiceParams, + InjectedMetadataParams, InjectedMetadataStartContract, } from './injected_metadata_service'; diff --git a/src/core/public/injected_metadata/injected_metadata_service.ts b/src/core/public/injected_metadata/injected_metadata_service.ts index 710e0c567ebbf..e82aab767593f 100644 --- a/src/core/public/injected_metadata/injected_metadata_service.ts +++ b/src/core/public/injected_metadata/injected_metadata_service.ts @@ -19,7 +19,7 @@ import { deepFreeze } from './deep_freeze'; -export interface InjectedMetadataServiceParams { +export interface InjectedMetadataParams { injectedMetadata: { legacyMetadata: { [key: string]: any; @@ -28,7 +28,7 @@ export interface InjectedMetadataServiceParams { } export class InjectedMetadataService { - constructor(private params: InjectedMetadataServiceParams) {} + constructor(private params: InjectedMetadataParams) {} public start() { const state = deepFreeze(this.params.injectedMetadata); diff --git a/src/core/public/legacy_platform/index.ts b/src/core/public/legacy_platform/index.ts index a5e3d698c8123..44862dcb4430a 100644 --- a/src/core/public/legacy_platform/index.ts +++ b/src/core/public/legacy_platform/index.ts @@ -17,4 +17,4 @@ * under the License. */ -export { LegacyPlatformService, LegacyPlatformServiceParams } from './legacy_platform_service'; +export { LegacyPlatformService, LegacyPlatformParams } from './legacy_platform_service'; diff --git a/src/core/public/legacy_platform/legacy_platform_service.ts b/src/core/public/legacy_platform/legacy_platform_service.ts index ec314cc04e275..bce3b5e1438df 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.ts @@ -23,14 +23,14 @@ interface Deps { injectedMetadata: InjectedMetadataStartContract; } -export interface LegacyPlatformServiceParams { +export interface LegacyPlatformParams { rootDomElement: HTMLElement; requireLegacyFiles: () => void; useLegacyTestHarness?: boolean; } export class LegacyPlatformService { - constructor(private params: LegacyPlatformServiceParams) {} + constructor(private params: LegacyPlatformParams) {} public start({ injectedMetadata }: Deps) { // Inject parts of the new platform into parts of the legacy platform From 33a74af3af3a896c73d032b5fb8e341d20009a06 Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 16 Jul 2018 15:12:45 -0700 Subject: [PATCH 18/30] [core/public] tweak tests to address feedback --- src/core/public/core_system.test.ts | 12 +++++++++--- .../injected_metadata_service.test.ts | 11 ----------- .../legacy_platform/legacy_platform_service.test.ts | 11 ----------- 3 files changed, 9 insertions(+), 25 deletions(-) diff --git a/src/core/public/core_system.test.ts b/src/core/public/core_system.test.ts index fbdd6dc999247..68148bc790e0b 100644 --- a/src/core/public/core_system.test.ts +++ b/src/core/public/core_system.test.ts @@ -100,16 +100,22 @@ describe('constructor', () => { }); describe('#start()', () => { - it('calls lifecycleSystem#start() and injectedMetadata#start()', () => { - const core = new CoreSystem({ + let core; + + beforeAll(() => { + core = new CoreSystem({ ...defaultCoreSystemParams, }); - expect(core.start()).toBe(undefined); + core.start(); + }); + it('calls injectedMetadata#start()', () => { expect(MockInjectedMetadataService.mock.instances[0].start).toHaveBeenCalledTimes(1); expect(MockInjectedMetadataService.mock.instances[0].start).toHaveBeenCalledWith(); + }); + it('calls lifecycleSystem#start()', () => { expect(MockLegacyPlatformService.mock.instances[0].start).toHaveBeenCalledTimes(1); expect(MockLegacyPlatformService.mock.instances[0].start).toHaveBeenCalledWith({ injectedMetadata: mockInjectedMetadataStartContract, diff --git a/src/core/public/injected_metadata/injected_metadata_service.test.ts b/src/core/public/injected_metadata/injected_metadata_service.test.ts index ad1e74efcee9b..59b3cc65db24e 100644 --- a/src/core/public/injected_metadata/injected_metadata_service.test.ts +++ b/src/core/public/injected_metadata/injected_metadata_service.test.ts @@ -20,17 +20,6 @@ import { InjectedMetadataService } from './injected_metadata_service'; describe('#start()', () => { - it('returns the start contract', () => { - const injectedMetadata = new InjectedMetadataService({ - injectedMetadata: {} as any, - }); - - const contract = injectedMetadata.start(); - expect(contract).toEqual({ - getLegacyMetadata: expect.any(Function), - }); - }); - it('deeply freezes its injectedMetadata param', () => { const params = { injectedMetadata: { foo: true } as any, diff --git a/src/core/public/legacy_platform/legacy_platform_service.test.ts b/src/core/public/legacy_platform/legacy_platform_service.test.ts index 3fc18913f6d92..0427e3274f731 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.test.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.test.ts @@ -65,17 +65,6 @@ beforeEach(() => { describe('#start()', () => { describe('default', () => { - it('does not return a start contract', () => { - const legacyPlatform = new LegacyPlatformService({ - ...defaultParams, - }); - - const startContract = legacyPlatform.start({ - injectedMetadata: injectedMetadataStartContract, - }); - expect(startContract).toBe(undefined); - }); - it('passes legacy metadata from injectedVars to ui/metadata', () => { const legacyMetadata = { isLegacyMetadata: true }; injectedMetadataStartContract.getLegacyMetadata.mockReturnValue(legacyMetadata); From 3e5a2abdd3f8789a7ce941b9d769bb666171d459 Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 16 Jul 2018 15:53:45 -0700 Subject: [PATCH 19/30] [core/public/core/tests] start the core once in each tests so mocks can be cleared --- src/core/public/core_system.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/public/core_system.test.ts b/src/core/public/core_system.test.ts index 68148bc790e0b..963c4afab3ba6 100644 --- a/src/core/public/core_system.test.ts +++ b/src/core/public/core_system.test.ts @@ -100,22 +100,22 @@ describe('constructor', () => { }); describe('#start()', () => { - let core; - - beforeAll(() => { - core = new CoreSystem({ + function startCore() { + const core = new CoreSystem({ ...defaultCoreSystemParams, }); core.start(); - }); + } it('calls injectedMetadata#start()', () => { + startCore(); expect(MockInjectedMetadataService.mock.instances[0].start).toHaveBeenCalledTimes(1); expect(MockInjectedMetadataService.mock.instances[0].start).toHaveBeenCalledWith(); }); it('calls lifecycleSystem#start()', () => { + startCore(); expect(MockLegacyPlatformService.mock.instances[0].start).toHaveBeenCalledTimes(1); expect(MockLegacyPlatformService.mock.instances[0].start).toHaveBeenCalledWith({ injectedMetadata: mockInjectedMetadataStartContract, From 0ec87936dcac88d0d546c116a0095d74baf3e47b Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 17 Jul 2018 17:17:05 -0700 Subject: [PATCH 20/30] [testBundle] use a Set() to avoid duplication --- src/core_plugins/tests_bundle/index.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/core_plugins/tests_bundle/index.js b/src/core_plugins/tests_bundle/index.js index bc9c736a0311a..82e42de477cdf 100644 --- a/src/core_plugins/tests_bundle/index.js +++ b/src/core_plugins/tests_bundle/index.js @@ -34,7 +34,7 @@ export default (kibana) => { uiExports: { async __bundleProvider__(kbnServer) { - const modules = []; + const modules = new Set(); const { config, @@ -64,8 +64,8 @@ export default (kibana) => { // add the modules from all of this plugins apps for (const app of uiApps) { - if (app.getPluginId() === pluginId && !modules.includes(app.getMainModuleId())) { - modules.push(app.getMainModuleId()); + if (app.getPluginId() === pluginId) { + modules.add(app.getMainModuleId()); } } @@ -74,9 +74,7 @@ export default (kibana) => { } else { // add the modules from all of the apps for (const app of uiApps) { - if (!modules.includes(app.getMainModuleId())) { - modules.push(app.getMainModuleId()); - } + modules.app(app.getMainModuleId()); } for (const plugin of plugins) { @@ -85,7 +83,7 @@ export default (kibana) => { } const testFiles = await findSourceFiles(testGlobs); - for (const f of testFiles) modules.push(f); + for (const f of testFiles) modules.add(f); if (config.get('tests_bundle.instrument')) { uiBundles.addPostLoader({ @@ -97,7 +95,7 @@ export default (kibana) => { uiBundles.add({ id: 'tests', - modules, + modules: [...modules], template: createTestEntryTemplate(uiSettingDefaults), }); }, From 794ae2c5425b8ba45563364cff943a20ad9523a0 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 17 Jul 2018 17:17:25 -0700 Subject: [PATCH 21/30] [core/public] add comments describing the core services/systems --- src/core/public/core_system.ts | 6 ++++++ .../public/injected_metadata/injected_metadata_service.ts | 6 ++++++ src/core/public/legacy_platform/legacy_platform_service.ts | 7 +++++++ 3 files changed, 19 insertions(+) diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 4b0e0919e6ca2..cc659d74e01af 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -27,6 +27,12 @@ interface Params { useLegacyTestHarness?: LegacyPlatformParams['useLegacyTestHarness']; } +/** + * The CoreSystem is the root of the new platform, and starts all parts + * of Kibana in the UI, including the LegacyPlatform which is managed + * by the LegacyPlatformService. As we migrate more things to the new + * platform the CoreSystem will get many more Services. + */ export class CoreSystem { private injectedMetadata: InjectedMetadataService; private legacyPlatform: LegacyPlatformService; diff --git a/src/core/public/injected_metadata/injected_metadata_service.ts b/src/core/public/injected_metadata/injected_metadata_service.ts index e82aab767593f..84900c8e2d86b 100644 --- a/src/core/public/injected_metadata/injected_metadata_service.ts +++ b/src/core/public/injected_metadata/injected_metadata_service.ts @@ -27,6 +27,12 @@ export interface InjectedMetadataParams { }; } +/** + * Provides access to the metadata that is injected by the + * server into the page. The metadata is actually defined + * in the entry file for the bundle containing the new platform + * and is read from the DOM in most cases. + */ export class InjectedMetadataService { constructor(private params: InjectedMetadataParams) {} diff --git a/src/core/public/legacy_platform/legacy_platform_service.ts b/src/core/public/legacy_platform/legacy_platform_service.ts index bce3b5e1438df..00b7427f40be7 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.ts @@ -29,6 +29,13 @@ export interface LegacyPlatformParams { useLegacyTestHarness?: boolean; } +/** + * The LegacyPlatformService is responsible for initializing + * the legacy platform by injecting parts of the new platform + * services into the legacy platform modules, like ui/modules, + * and then bootstraping the ui/chrome or ui/test_harness to + * start either the app or browser tests. + */ export class LegacyPlatformService { constructor(private params: LegacyPlatformParams) {} From fad92a0ad248d0f3ffca1464384fbcb3e30508f4 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 17 Jul 2018 17:18:24 -0700 Subject: [PATCH 22/30] [core/public] use `readonly` modifier for params property --- src/core/public/injected_metadata/injected_metadata_service.ts | 2 +- src/core/public/legacy_platform/legacy_platform_service.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/public/injected_metadata/injected_metadata_service.ts b/src/core/public/injected_metadata/injected_metadata_service.ts index 84900c8e2d86b..2921d97fca66d 100644 --- a/src/core/public/injected_metadata/injected_metadata_service.ts +++ b/src/core/public/injected_metadata/injected_metadata_service.ts @@ -34,7 +34,7 @@ export interface InjectedMetadataParams { * and is read from the DOM in most cases. */ export class InjectedMetadataService { - constructor(private params: InjectedMetadataParams) {} + constructor(private readonly params: InjectedMetadataParams) {} public start() { const state = deepFreeze(this.params.injectedMetadata); diff --git a/src/core/public/legacy_platform/legacy_platform_service.ts b/src/core/public/legacy_platform/legacy_platform_service.ts index 00b7427f40be7..ed7976bfa2a35 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.ts @@ -37,7 +37,7 @@ export interface LegacyPlatformParams { * start either the app or browser tests. */ export class LegacyPlatformService { - constructor(private params: LegacyPlatformParams) {} + constructor(private readonly params: LegacyPlatformParams) {} public start({ injectedMetadata }: Deps) { // Inject parts of the new platform into parts of the legacy platform From 7442290428615e319856ce7a14d9ec9898b90b08 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 17 Jul 2018 17:22:01 -0700 Subject: [PATCH 23/30] [uiBundles] add comment about isCacheValid() --- src/ui/ui_bundles/ui_bundle.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/ui/ui_bundles/ui_bundle.js b/src/ui/ui_bundles/ui_bundle.js index 015305bba0d3d..200235af633a2 100644 --- a/src/ui/ui_bundles/ui_bundle.js +++ b/src/ui/ui_bundles/ui_bundle.js @@ -92,6 +92,17 @@ export class UiBundle { )); } + /** + * Determine if the cache for this bundle is valid by + * checking that the entry file exists, has the content we + * expect based on the argument for this bundle, and that both + * the style file and output for this bundle exist. In this + * scenario we assume the cache is valid. + * + * When the `optimize.useBundleCache` config is set to `false` + * (the default when running in development) we don't even call + * this method and bundles are always recreated. + */ async isCacheValid() { if (await this.readEntryFile() !== this.renderContent()) { return false; From 4567aed7317ea61e5511edf1c0d58f0c3ae6560f Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 17 Jul 2018 17:22:35 -0700 Subject: [PATCH 24/30] [eslintResolver] remove ui_framework alias from webpack config --- .../kbn-eslint-import-resolver-kibana/lib/get_webpack_config.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/kbn-eslint-import-resolver-kibana/lib/get_webpack_config.js b/packages/kbn-eslint-import-resolver-kibana/lib/get_webpack_config.js index f0425026d1fdb..9ba116c66238a 100755 --- a/packages/kbn-eslint-import-resolver-kibana/lib/get_webpack_config.js +++ b/packages/kbn-eslint-import-resolver-kibana/lib/get_webpack_config.js @@ -28,7 +28,6 @@ exports.getWebpackConfig = function(kibanaPath, projectRoot, config) { const alias = { // Kibana defaults https://github.com/elastic/kibana/blob/6998f074542e8c7b32955db159d15661aca253d7/src/ui/ui_bundler_env.js#L30-L36 ui: fromKibana('src/ui/public'), - ui_framework: fromKibana('ui_framework'), test_harness: fromKibana('src/test_harness/public'), querystring: 'querystring-browser', moment$: fromKibana('webpackShims/moment'), From 4f9f64db92cdb83465d791a4d136bbd7388c1c5f Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 17 Jul 2018 17:24:09 -0700 Subject: [PATCH 25/30] fix references to remove kbn-initial-state element --- src/core_plugins/console/public/tests/src/integration.test.js | 2 +- src/core_plugins/state_session_storage_redirect/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core_plugins/console/public/tests/src/integration.test.js b/src/core_plugins/console/public/tests/src/integration.test.js index 126c01a9e14d8..410e933dfafd3 100644 --- a/src/core_plugins/console/public/tests/src/integration.test.js +++ b/src/core_plugins/console/public/tests/src/integration.test.js @@ -33,7 +33,7 @@ describe('Integration', () => { beforeEach(() => { // Set up our document body document.body.innerHTML = - '
'; + '
'; input = initializeInput( $('#editor'), diff --git a/src/core_plugins/state_session_storage_redirect/package.json b/src/core_plugins/state_session_storage_redirect/package.json index 7eedbddb08325..21956e5d76d5b 100644 --- a/src/core_plugins/state_session_storage_redirect/package.json +++ b/src/core_plugins/state_session_storage_redirect/package.json @@ -1,5 +1,5 @@ { "name": "state_session_storage_redirect", "version": "kibana", - "description": "When using the state:storeInSessionStorage setting with the short-urls, we need some way to get the full URL's hashed states into sessionStorage, this app will grab the URL from the kbn-initial-state and and put the URL hashed states into sessionStorage before redirecting the user." + "description": "When using the state:storeInSessionStorage setting with the short-urls, we need some way to get the full URL's hashed states into sessionStorage, this app will grab the URL from the injected state and and put the URL hashed states into sessionStorage before redirecting the user." } From 874029c5cdd26ad9ca7c78557bfad7f2394a4cf4 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 17 Jul 2018 17:25:11 -0700 Subject: [PATCH 26/30] [core/public/tests] use destructuring for readability --- src/core/public/core_system.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/core/public/core_system.test.ts b/src/core/public/core_system.test.ts index 963c4afab3ba6..c33b4c5c827c9 100644 --- a/src/core/public/core_system.test.ts +++ b/src/core/public/core_system.test.ts @@ -110,14 +110,16 @@ describe('#start()', () => { it('calls injectedMetadata#start()', () => { startCore(); - expect(MockInjectedMetadataService.mock.instances[0].start).toHaveBeenCalledTimes(1); - expect(MockInjectedMetadataService.mock.instances[0].start).toHaveBeenCalledWith(); + const [mockInstance] = MockInjectedMetadataService.mock.instances; + expect(mockInstance.start).toHaveBeenCalledTimes(1); + expect(mockInstance.start).toHaveBeenCalledWith(); }); it('calls lifecycleSystem#start()', () => { startCore(); - expect(MockLegacyPlatformService.mock.instances[0].start).toHaveBeenCalledTimes(1); - expect(MockLegacyPlatformService.mock.instances[0].start).toHaveBeenCalledWith({ + const [mockInstance] = MockInjectedMetadataService.mock.instances; + expect(mockInstance.start).toHaveBeenCalledTimes(1); + expect(mockInstance.start).toHaveBeenCalledWith({ injectedMetadata: mockInjectedMetadataStartContract, }); }); From c210fefb8482894e89c9c44ca35a419794b7b794 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 17 Jul 2018 17:26:08 -0700 Subject: [PATCH 27/30] [core/public/tests] cleanup in after hook for clarity --- src/core/public/legacy_platform/legacy_platform_service.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/public/legacy_platform/legacy_platform_service.test.ts b/src/core/public/legacy_platform/legacy_platform_service.test.ts index 0427e3274f731..3927d7f56149d 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.test.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.test.ts @@ -56,7 +56,7 @@ const defaultParams = { }), }; -beforeEach(() => { +afterEach(() => { jest.clearAllMocks(); injectedMetadataStartContract.getLegacyMetadata.mockReset(); jest.resetModules(); From 7a4987f0ed8394fdcbe890fa4357d408bc9120c5 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 17 Jul 2018 17:27:43 -0700 Subject: [PATCH 28/30] [core/public/deepFreeze] test that freezing arrays is supported --- .../public/injected_metadata/deep_freeze.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/core/public/injected_metadata/deep_freeze.test.ts b/src/core/public/injected_metadata/deep_freeze.test.ts index 7d93a97ad73de..9086d697f9ce3 100644 --- a/src/core/public/injected_metadata/deep_freeze.test.ts +++ b/src/core/public/injected_metadata/deep_freeze.test.ts @@ -58,6 +58,22 @@ it('prevents changing properties on nested children of argument', () => { }).toThrowError(`read only property 'box'`); }); +it('prevents adding items to a frozen array', () => { + const frozen = deepFreeze({ foo: [1] }); + expect(() => { + // @ts-ignore ts knows this shouldn't be possible, but just making sure + frozen.foo.push(2); + }).toThrowError(`object is not extensible`); +}); + +it('prevents reassigning items in a frozen array', () => { + const frozen = deepFreeze({ foo: [1] }); + expect(() => { + // @ts-ignore ts knows this shouldn't be possible, but just making sure + frozen.foo[0] = 2; + }).toThrowError(`read only property '0'`); +}); + it('types return values to prevent mutations in typescript', async () => { const result = await execa.stdout( 'tsc', From 5b0edf1a00b87016ed95ca0347c49a5c86ada0bf Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 17 Jul 2018 17:55:41 -0700 Subject: [PATCH 29/30] [testsBundle] fix typo --- src/core_plugins/tests_bundle/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core_plugins/tests_bundle/index.js b/src/core_plugins/tests_bundle/index.js index 82e42de477cdf..b78995ecfe649 100644 --- a/src/core_plugins/tests_bundle/index.js +++ b/src/core_plugins/tests_bundle/index.js @@ -74,7 +74,7 @@ export default (kibana) => { } else { // add the modules from all of the apps for (const app of uiApps) { - modules.app(app.getMainModuleId()); + modules.add(app.getMainModuleId()); } for (const plugin of plugins) { From 9710387b94d86e41d05d1f0ffb80ce6be1c72a5d Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 17 Jul 2018 18:24:22 -0700 Subject: [PATCH 30/30] [core/public/tests] fix typo --- src/core/public/core_system.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/public/core_system.test.ts b/src/core/public/core_system.test.ts index c33b4c5c827c9..76d615b3a45e0 100644 --- a/src/core/public/core_system.test.ts +++ b/src/core/public/core_system.test.ts @@ -117,7 +117,7 @@ describe('#start()', () => { it('calls lifecycleSystem#start()', () => { startCore(); - const [mockInstance] = MockInjectedMetadataService.mock.instances; + const [mockInstance] = MockLegacyPlatformService.mock.instances; expect(mockInstance.start).toHaveBeenCalledTimes(1); expect(mockInstance.start).toHaveBeenCalledWith({ injectedMetadata: mockInjectedMetadataStartContract,