Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core/ui] bootstrap the legacy platform within the new platform #20699

Merged
merged 37 commits into from
Jul 18, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
97c86a3
[core/ui] bootstrap the legacy platform within the new platform
Jul 11, 2018
a7fd8fb
fix test that parsed uiApp response
Jul 11, 2018
bb73252
fix missed rename
Jul 11, 2018
0a8dcec
use a legacy platform service to bootstrap within the start() lifecycle
Jul 12, 2018
f1a698e
[tests/ui_exports_replace_injected_vars] read vars from legacyMetadata
Jul 12, 2018
9984fa5
[core/injectedMetadata] use less permissive array type to avoid freez…
Jul 12, 2018
6624b58
[ui/metadata/tests] remove unnecessary test
Jul 12, 2018
e514979
Merge branch 'master' of github.com:elastic/kibana into implement/new…
Jul 12, 2018
4f8fe21
implement feedback from Court
Jul 12, 2018
5357110
restore app_entry_template tests
Jul 12, 2018
3c71d11
Merge branch 'master' of github.com:elastic/kibana into implement/new…
Jul 13, 2018
b662c4d
[ui/notify/tests] remove reference to window.__KBN__
Jul 13, 2018
277f273
Merge branch 'master' of github.com:elastic/kibana into implement/new…
Jul 13, 2018
e404fa4
[core] use *StartContract naming convention
Jul 13, 2018
cc1d8f0
Merge branch 'master' of github.com:elastic/kibana into implement/new…
Jul 13, 2018
2151adf
[tslint] ignore fixtures in precommit hook
Jul 13, 2018
04af61a
[core/public] add tests, use *Params in constructors
Jul 13, 2018
9c76bfb
[core/public/tests] fix type definitions
Jul 13, 2018
3073159
[core/public/legacyPlatform] test arguments to legacy bootstrap module
Jul 13, 2018
450e6b1
[core/public/core/tests] use correct services for mock generics
Jul 13, 2018
5a5210b
Merge branch 'master' of github.com:elastic/kibana into implement/new…
Jul 13, 2018
675f032
[core/public] list explicit params for core service
Jul 13, 2018
7ec434b
Merge branch 'master' of github.com:elastic/kibana into implement/new…
Jul 16, 2018
33a74af
[core/public] tweak tests to address feedback
Jul 16, 2018
3e5a2ab
[core/public/core/tests] start the core once in each tests so mocks c…
Jul 16, 2018
e6c5c0b
Merge branch 'master' of github.com:elastic/kibana into implement/new…
Jul 18, 2018
0ec8793
[testBundle] use a Set() to avoid duplication
Jul 18, 2018
794ae2c
[core/public] add comments describing the core services/systems
Jul 18, 2018
fad92a0
[core/public] use `readonly` modifier for params property
Jul 18, 2018
7442290
[uiBundles] add comment about isCacheValid()
Jul 18, 2018
4567aed
[eslintResolver] remove ui_framework alias from webpack config
Jul 18, 2018
4f9f64d
fix references to remove kbn-initial-state element
Jul 18, 2018
874029c
[core/public/tests] use destructuring for readability
Jul 18, 2018
c210fef
[core/public/tests] cleanup in after hook for clarity
Jul 18, 2018
7a4987f
[core/public/deepFreeze] test that freezing arrays is supported
Jul 18, 2018
5b0edf1
[testsBundle] fix typo
Jul 18, 2018
9710387
[core/public/tests] fix typo
Jul 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions src/core/public/core_system.ts
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would be really great if you can add a top level comment for CoreSystem explaining what its role and what it's supposed to do.

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();
}
}
20 changes: 20 additions & 0 deletions src/core/public/index.ts
Original file line number Diff line number Diff line change
@@ -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';
36 changes: 36 additions & 0 deletions src/core/public/injected_metadata/deep_freeze.ts
Original file line number Diff line number Diff line change
@@ -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<any>;

type RecursiveReadOnly<T> = T extends Freezable
? Readonly<{ [K in keyof T]: RecursiveReadOnly<T[K]> }>
: T;

export function deepFreeze<T extends Freezable>(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<T>;
}
20 changes: 20 additions & 0 deletions src/core/public/injected_metadata/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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 <kbn-injected-metadata> element');
}

const json = el.getAttribute('data');
if (!json) {
throw new Error('<kbn-injected-metadata> does not have a data atttibute');
}

return JSON.parse(json);
}
12 changes: 6 additions & 6 deletions src/core_plugins/tests_bundle/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
* under the License.
*/

import { union } from 'lodash';

import { fromRoot } from '../../utils';

import findSourceFiles from './find_source_files';
Expand All @@ -36,7 +34,7 @@ export default (kibana) => {

uiExports: {
async __bundleProvider__(kbnServer) {
let modules = [];
const modules = [];

const {
config,
Expand Down Expand Up @@ -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())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional nit: if we use const modules = new Set() we won't need this !....includes checks (that we can accidentally forget about if we decide to add new for for some reason). The only cons is that we'll have to do modules: [...modules] or Array.from(modules) later.

modules.push(app.getMainModuleId());
}
}

Expand All @@ -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) {
Expand Down
16 changes: 12 additions & 4 deletions src/core_plugins/tests_bundle/tests_entry_template.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ export const createTestEntryTemplate = (defaultUiSettings) => (bundle) => `
*
*/

window.__KBN__ = {
import { CoreSystem } from '__kibanaCore__'

const legacyMetadata = {
version: '1.2.3',
buildNum: 1234,
vars: {
Expand Down Expand Up @@ -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(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The convention we've talked about is having a core start function and then invoking legacy startup/bootstrap through a legacy service within the core system. This way we can identify precisely when the legacy code kicks off in the context of all the new platform services we'd be rolling out.

Is there a reason we need to abandon that approach for this initLegacyPlatform approach?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, happy to move it there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just didn't want to worry about start()/stop() right now, but it's not a big deal.

require('ui/test_harness');
${bundle.getRequires().join('\n ')}
require('ui/test_harness').bootstrap();
})
`;
8 changes: 2 additions & 6 deletions src/optimize/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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,
Expand Down
5 changes: 1 addition & 4 deletions src/optimize/watch/watch_optimizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/ui/__tests__/ui_exports_replace_injected_vars.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
3 changes: 0 additions & 3 deletions src/ui/public/chrome/chrome.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
29 changes: 6 additions & 23 deletions src/ui/public/metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional nit: maybe get rid of else?

export function __newPlatformInit__(legacyMetadata) {
  if (metadata !== null) {
    throw new Error('ui/metadata can only be initialized once');
  }

  metadata = legacyMetadata;
}

if (metadata === null) {
metadata = legacyMetadata;
} else {
throw new Error('ui/metadata can only be initialized once');
}
return window[stateKey];
}
2 changes: 0 additions & 2 deletions src/ui/public/test_harness/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,4 @@
* under the License.
*/

import './test_harness';

export { bootstrap } from './test_harness';
Loading