From e9afa9bedd9f5cad5c2760030a64737519a6c390 Mon Sep 17 00:00:00 2001 From: Spencer Date: Tue, 28 Apr 2020 17:53:48 -0700 Subject: [PATCH] Consolidate downloading plugin bundles to bootstrap script (#64685) --- src/core/public/plugins/plugin.test.mocks.ts | 6 +- src/core/public/plugins/plugin.test.ts | 47 ++---- src/core/public/plugins/plugin.ts | 28 ++-- src/core/public/plugins/plugin_loader.mock.ts | 33 ---- src/core/public/plugins/plugin_loader.test.ts | 125 ---------------- src/core/public/plugins/plugin_loader.ts | 141 ------------------ src/core/public/plugins/plugin_reader.test.ts | 53 +++++++ src/core/public/plugins/plugin_reader.ts | 52 +++++++ .../plugins/plugins_service.test.mocks.ts | 10 +- .../public/plugins/plugins_service.test.ts | 35 +---- src/core/public/plugins/plugins_service.ts | 8 - src/core/server/rendering/views/template.tsx | 4 + src/legacy/server/kbn_server.d.ts | 1 + .../ui/ui_render/bootstrap/template.js.hbs | 6 +- src/legacy/ui/ui_render/ui_render_mixin.js | 33 ++-- 15 files changed, 169 insertions(+), 413 deletions(-) delete mode 100644 src/core/public/plugins/plugin_loader.mock.ts delete mode 100644 src/core/public/plugins/plugin_loader.test.ts delete mode 100644 src/core/public/plugins/plugin_loader.ts create mode 100644 src/core/public/plugins/plugin_reader.test.ts create mode 100644 src/core/public/plugins/plugin_reader.ts diff --git a/src/core/public/plugins/plugin.test.mocks.ts b/src/core/public/plugins/plugin.test.mocks.ts index b877847aaa90ed..422442c9ca4d2a 100644 --- a/src/core/public/plugins/plugin.test.mocks.ts +++ b/src/core/public/plugins/plugin.test.mocks.ts @@ -24,8 +24,8 @@ export const mockPlugin = { }; export const mockInitializer = jest.fn(() => mockPlugin); -export const mockPluginLoader = jest.fn().mockResolvedValue(mockInitializer); +export const mockPluginReader = jest.fn(() => mockInitializer); -jest.mock('./plugin_loader', () => ({ - loadPluginBundle: mockPluginLoader, +jest.mock('./plugin_reader', () => ({ + read: mockPluginReader, })); diff --git a/src/core/public/plugins/plugin.test.ts b/src/core/public/plugins/plugin.test.ts index 39330711f79809..8fe745db9554df 100644 --- a/src/core/public/plugins/plugin.test.ts +++ b/src/core/public/plugins/plugin.test.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { mockInitializer, mockPlugin, mockPluginLoader } from './plugin.test.mocks'; +import { mockInitializer, mockPlugin, mockPluginReader } from './plugin.test.mocks'; import { DiscoveredPlugin } from '../../server'; import { coreMock } from '../mocks'; @@ -38,10 +38,9 @@ function createManifest( let plugin: PluginWrapper>; const opaqueId = Symbol(); const initializerContext = coreMock.createPluginInitializerContext(); -const addBasePath = (path: string) => path; beforeEach(() => { - mockPluginLoader.mockClear(); + mockPluginReader.mockClear(); mockPlugin.setup.mockClear(); mockPlugin.start.mockClear(); mockPlugin.stop.mockClear(); @@ -49,20 +48,8 @@ beforeEach(() => { }); describe('PluginWrapper', () => { - test('`load` calls loadPluginBundle', () => { - plugin.load(addBasePath); - expect(mockPluginLoader).toHaveBeenCalledWith(addBasePath, 'plugin-a'); - }); - - test('`setup` fails if load is not called first', async () => { - await expect(plugin.setup({} as any, {} as any)).rejects.toThrowErrorMatchingInlineSnapshot( - `"Plugin \\"plugin-a\\" can't be setup since its bundle isn't loaded."` - ); - }); - test('`setup` fails if plugin.setup is not a function', async () => { mockInitializer.mockReturnValueOnce({ start: jest.fn() } as any); - await plugin.load(addBasePath); await expect(plugin.setup({} as any, {} as any)).rejects.toThrowErrorMatchingInlineSnapshot( `"Instance of plugin \\"plugin-a\\" does not define \\"setup\\" function."` ); @@ -70,20 +57,17 @@ describe('PluginWrapper', () => { test('`setup` fails if plugin.start is not a function', async () => { mockInitializer.mockReturnValueOnce({ setup: jest.fn() } as any); - await plugin.load(addBasePath); await expect(plugin.setup({} as any, {} as any)).rejects.toThrowErrorMatchingInlineSnapshot( `"Instance of plugin \\"plugin-a\\" does not define \\"start\\" function."` ); }); test('`setup` calls initializer with initializer context', async () => { - await plugin.load(addBasePath); await plugin.setup({} as any, {} as any); expect(mockInitializer).toHaveBeenCalledWith(initializerContext); }); test('`setup` calls plugin.setup with context and dependencies', async () => { - await plugin.load(addBasePath); const context = { any: 'thing' } as any; const deps = { otherDep: 'value' }; await plugin.setup(context, deps); @@ -91,14 +75,12 @@ describe('PluginWrapper', () => { }); test('`start` fails if setup is not called first', async () => { - await plugin.load(addBasePath); await expect(plugin.start({} as any, {} as any)).rejects.toThrowErrorMatchingInlineSnapshot( `"Plugin \\"plugin-a\\" can't be started since it isn't set up."` ); }); test('`start` calls plugin.start with context and dependencies', async () => { - await plugin.load(addBasePath); await plugin.setup({} as any, {} as any); const context = { any: 'thing' } as any; const deps = { otherDep: 'value' }; @@ -114,20 +96,21 @@ describe('PluginWrapper', () => { }; let startDependenciesResolved = false; - mockPluginLoader.mockResolvedValueOnce(() => ({ - setup: jest.fn(), - start: async () => { - // Add small delay to ensure startDependencies is not resolved until after the plugin instance's start resolves. - await new Promise(resolve => setTimeout(resolve, 10)); - expect(startDependenciesResolved).toBe(false); - return pluginStartContract; - }, - })); - await plugin.load(addBasePath); + mockPluginReader.mockReturnValueOnce( + jest.fn(() => ({ + setup: jest.fn(), + start: jest.fn(async () => { + // Add small delay to ensure startDependencies is not resolved until after the plugin instance's start resolves. + await new Promise(resolve => setTimeout(resolve, 10)); + expect(startDependenciesResolved).toBe(false); + return pluginStartContract; + }), + stop: jest.fn(), + })) + ); await plugin.setup({} as any, {} as any); const context = { any: 'thing' } as any; const deps = { otherDep: 'value' }; - // Add promise callback prior to calling `start` to ensure calls in `setup` will not resolve before `start` is // called. const startDependenciesCheck = plugin.startDependencies.then(res => { @@ -145,7 +128,6 @@ describe('PluginWrapper', () => { }); test('`stop` calls plugin.stop', async () => { - await plugin.load(addBasePath); await plugin.setup({} as any, {} as any); await plugin.stop(); expect(mockPlugin.stop).toHaveBeenCalled(); @@ -153,7 +135,6 @@ describe('PluginWrapper', () => { test('`stop` does not fail if plugin.stop does not exist', async () => { mockInitializer.mockReturnValueOnce({ setup: jest.fn(), start: jest.fn() } as any); - await plugin.load(addBasePath); await plugin.setup({} as any, {} as any); expect(() => plugin.stop()).not.toThrow(); }); diff --git a/src/core/public/plugins/plugin.ts b/src/core/public/plugins/plugin.ts index e51c45040c452e..591165fcd28390 100644 --- a/src/core/public/plugins/plugin.ts +++ b/src/core/public/plugins/plugin.ts @@ -21,7 +21,7 @@ import { Subject } from 'rxjs'; import { first } from 'rxjs/operators'; import { DiscoveredPlugin, PluginOpaqueId } from '../../server'; import { PluginInitializerContext } from './plugin_context'; -import { loadPluginBundle } from './plugin_loader'; +import { read } from './plugin_reader'; import { CoreStart, CoreSetup } from '..'; /** @@ -69,7 +69,6 @@ export class PluginWrapper< public readonly configPath: DiscoveredPlugin['configPath']; public readonly requiredPlugins: DiscoveredPlugin['requiredPlugins']; public readonly optionalPlugins: DiscoveredPlugin['optionalPlugins']; - private initializer?: PluginInitializer; private instance?: Plugin; private readonly startDependencies$ = new Subject<[CoreStart, TPluginsStart, TStart]>(); @@ -86,18 +85,6 @@ export class PluginWrapper< this.optionalPlugins = discoveredPlugin.optionalPlugins; } - /** - * Loads the plugin's bundle into the browser. Should be called in parallel with all plugins - * using `Promise.all`. Must be called before `setup`. - * @param addBasePath Function that adds the base path to a string for plugin bundle path. - */ - public async load(addBasePath: (path: string) => string) { - this.initializer = await loadPluginBundle( - addBasePath, - this.name - ); - } - /** * Instantiates plugin and calls `setup` function exposed by the plugin initializer. * @param setupContext Context that consists of various core services tailored specifically @@ -146,11 +133,14 @@ export class PluginWrapper< } private async createPluginInstance() { - if (this.initializer === undefined) { - throw new Error(`Plugin "${this.name}" can't be setup since its bundle isn't loaded.`); - } - - const instance = this.initializer(this.initializerContext); + const initializer = read(this.name) as PluginInitializer< + TSetup, + TStart, + TPluginsSetup, + TPluginsStart + >; + + const instance = initializer(this.initializerContext); if (typeof instance.setup !== 'function') { throw new Error(`Instance of plugin "${this.name}" does not define "setup" function.`); diff --git a/src/core/public/plugins/plugin_loader.mock.ts b/src/core/public/plugins/plugin_loader.mock.ts deleted file mode 100644 index abdd9d4ddce2ab..00000000000000 --- a/src/core/public/plugins/plugin_loader.mock.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. - */ - -import { PluginName } from 'src/core/server'; -import { LoadPluginBundle, UnknownPluginInitializer } from './plugin_loader'; - -/** - * @param initializerProvider A function provided by the test to resolve initializers. - */ -const createLoadPluginBundleMock = ( - initializerProvider: (name: PluginName) => UnknownPluginInitializer -): jest.Mock, Parameters> => - jest.fn((addBasePath, pluginName, _ = {}) => { - return Promise.resolve(initializerProvider(pluginName)) as any; - }); - -export const loadPluginBundleMock = { create: createLoadPluginBundleMock }; diff --git a/src/core/public/plugins/plugin_loader.test.ts b/src/core/public/plugins/plugin_loader.test.ts deleted file mode 100644 index b4e2c3095f14a7..00000000000000 --- a/src/core/public/plugins/plugin_loader.test.ts +++ /dev/null @@ -1,125 +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. - */ - -import { CoreWindow, loadPluginBundle } from './plugin_loader'; - -let createdScriptTags = [] as any[]; -let appendChildSpy: jest.SpyInstance; -let createElementSpy: jest.SpyInstance< - HTMLElement, - [string, (ElementCreationOptions | undefined)?] ->; - -const coreWindow = (window as unknown) as CoreWindow; - -beforeEach(() => { - // Mock document.createElement to return fake tags we can use to inspect what - // loadPluginBundles does. - createdScriptTags = []; - createElementSpy = jest.spyOn(document, 'createElement').mockImplementation(() => { - const scriptTag = { setAttribute: jest.fn() } as any; - createdScriptTags.push(scriptTag); - return scriptTag; - }); - - // Mock document.body.appendChild to avoid errors about appending objects that aren't `Node`'s - // and so we can verify that the script tags were added to the page. - appendChildSpy = jest.spyOn(document.body, 'appendChild').mockReturnValue({} as any); - - // Mock global fields needed for loading modules. - coreWindow.__kbnBundles__ = {}; -}); - -afterEach(() => { - appendChildSpy.mockRestore(); - createElementSpy.mockRestore(); - delete coreWindow.__kbnBundles__; -}); - -const addBasePath = (path: string) => path; - -test('`loadPluginBundles` creates a script tag and loads initializer', async () => { - const loadPromise = loadPluginBundle(addBasePath, 'plugin-a'); - - // Verify it sets up the script tag correctly and adds it to document.body - expect(createdScriptTags).toHaveLength(1); - const fakeScriptTag = createdScriptTags[0]; - expect(fakeScriptTag.setAttribute).toHaveBeenCalledWith( - 'src', - '/bundles/plugin/plugin-a/plugin-a.plugin.js' - ); - expect(fakeScriptTag.setAttribute).toHaveBeenCalledWith('id', 'kbn-plugin-plugin-a'); - expect(fakeScriptTag.onload).toBeInstanceOf(Function); - expect(fakeScriptTag.onerror).toBeInstanceOf(Function); - expect(appendChildSpy).toHaveBeenCalledWith(fakeScriptTag); - - // Setup a fake initializer as if a plugin bundle had actually been loaded. - const fakeInitializer = jest.fn(); - coreWindow.__kbnBundles__['plugin/plugin-a'] = { plugin: fakeInitializer }; - // Call the onload callback - fakeScriptTag.onload(); - await expect(loadPromise).resolves.toEqual(fakeInitializer); -}); - -test('`loadPluginBundles` includes the basePath', async () => { - loadPluginBundle((path: string) => `/mybasepath${path}`, 'plugin-a'); - - // Verify it sets up the script tag correctly and adds it to document.body - expect(createdScriptTags).toHaveLength(1); - const fakeScriptTag = createdScriptTags[0]; - expect(fakeScriptTag.setAttribute).toHaveBeenCalledWith( - 'src', - '/mybasepath/bundles/plugin/plugin-a/plugin-a.plugin.js' - ); -}); - -test('`loadPluginBundles` rejects if script.onerror is called', async () => { - const loadPromise = loadPluginBundle(addBasePath, 'plugin-a'); - const fakeScriptTag1 = createdScriptTags[0]; - // Call the error on the second script - fakeScriptTag1.onerror(new Error('Whoa there!')); - - await expect(loadPromise).rejects.toThrowErrorMatchingInlineSnapshot( - `"Failed to load \\"plugin-a\\" bundle (/bundles/plugin/plugin-a/plugin-a.plugin.js)"` - ); -}); - -test('`loadPluginBundles` rejects if timeout is reached', async () => { - await expect( - // Override the timeout to 1 ms for testi. - loadPluginBundle(addBasePath, 'plugin-a', { timeoutMs: 1 }) - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"Timeout reached when loading \\"plugin-a\\" bundle (/bundles/plugin/plugin-a/plugin-a.plugin.js)"` - ); -}); - -test('`loadPluginBundles` rejects if bundle does attach an initializer to window.__kbnBundles__', async () => { - const loadPromise = loadPluginBundle(addBasePath, 'plugin-a'); - - const fakeScriptTag1 = createdScriptTags[0]; - - // Setup a fake initializer as if a plugin bundle had actually been loaded. - coreWindow.__kbnBundles__['plugin/plugin-a'] = undefined; - // Call the onload callback - fakeScriptTag1.onload(); - - await expect(loadPromise).rejects.toThrowErrorMatchingInlineSnapshot( - `"Definition of plugin \\"plugin-a\\" should be a function (/bundles/plugin/plugin-a/plugin-a.plugin.js)."` - ); -}); diff --git a/src/core/public/plugins/plugin_loader.ts b/src/core/public/plugins/plugin_loader.ts deleted file mode 100644 index bf7711055e97ba..00000000000000 --- a/src/core/public/plugins/plugin_loader.ts +++ /dev/null @@ -1,141 +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. - */ - -import { PluginName } from '../../server'; -import { PluginInitializer } from './plugin'; - -/** - * Unknown variant for internal use only for when plugins are not known. - * @internal - */ -export type UnknownPluginInitializer = PluginInitializer>; - -/** - * Custom window type for loading bundles. Do not extend global Window to avoid leaking these types. - * @internal - */ -export interface CoreWindow { - __kbnBundles__: { - [pluginBundleName: string]: { plugin: UnknownPluginInitializer } | undefined; - }; -} - -/** - * Timeout for loading a single script in milliseconds. - * @internal - */ -export const LOAD_TIMEOUT = 120 * 1000; // 2 minutes - -/** - * Loads the bundle for a plugin onto the page and returns their PluginInitializer. This should - * be called for all plugins (once per plugin) in parallel using Promise.all. - * - * If this is slowing down browser load time, there are some ways we could make this faster: - * - Add these bundles in the generated bootstrap.js file so they're loaded immediately - * - Concatenate all the bundles files on the backend and serve them in single request. - * - Use HTTP/2 to load these bundles without having to open new connections for each. - * - * This may not be much of an issue since these should be cached by the browser after the first - * page load. - * - * @param basePath - * @param plugins - * @internal - */ -export const loadPluginBundle: LoadPluginBundle = < - TSetup, - TStart, - TPluginsSetup extends object, - TPluginsStart extends object ->( - addBasePath: (path: string) => string, - pluginName: PluginName, - { timeoutMs = LOAD_TIMEOUT }: { timeoutMs?: number } = {} -) => - new Promise>( - (resolve, reject) => { - const coreWindow = (window as unknown) as CoreWindow; - const exportId = `plugin/${pluginName}`; - - const readPluginExport = () => { - const PluginExport: any = coreWindow.__kbnBundles__[exportId]; - if (typeof PluginExport?.plugin !== 'function') { - reject( - new Error(`Definition of plugin "${pluginName}" should be a function (${bundlePath}).`) - ); - } else { - resolve( - PluginExport.plugin as PluginInitializer - ); - } - }; - - if (coreWindow.__kbnBundles__[exportId]) { - readPluginExport(); - return; - } - - const script = document.createElement('script'); - // Assumes that all plugin bundles get put into the bundles/plugins subdirectory - const bundlePath = addBasePath(`/bundles/plugin/${pluginName}/${pluginName}.plugin.js`); - script.setAttribute('src', bundlePath); - script.setAttribute('id', `kbn-plugin-${pluginName}`); - script.setAttribute('async', ''); - - const cleanupTag = () => { - clearTimeout(timeout); - // Set to null for IE memory leak issue. Webpack does the same thing. - // @ts-ignore - script.onload = script.onerror = null; - }; - - // Wire up resolve and reject - script.onload = () => { - cleanupTag(); - readPluginExport(); - }; - - script.onerror = () => { - cleanupTag(); - reject(new Error(`Failed to load "${pluginName}" bundle (${bundlePath})`)); - }; - - const timeout = setTimeout(() => { - cleanupTag(); - reject(new Error(`Timeout reached when loading "${pluginName}" bundle (${bundlePath})`)); - }, timeoutMs); - - // Add the script tag to the end of the body to start downloading - document.body.appendChild(script); - } - ); - -/** - * @internal - */ -export type LoadPluginBundle = < - TSetup, - TStart, - TPluginsSetup extends object, - TPluginsStart extends object ->( - addBasePath: (path: string) => string, - pluginName: PluginName, - options?: { timeoutMs?: number } -) => Promise>; diff --git a/src/core/public/plugins/plugin_reader.test.ts b/src/core/public/plugins/plugin_reader.test.ts new file mode 100644 index 00000000000000..d4324f81de8e64 --- /dev/null +++ b/src/core/public/plugins/plugin_reader.test.ts @@ -0,0 +1,53 @@ +/* + * 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 { CoreWindow, read, UnknownPluginInitializer } from './plugin_reader'; + +const coreWindow: CoreWindow = window as any; +beforeEach(() => { + coreWindow.__kbnBundles__ = {}; +}); + +it('handles undefined plugin exports', () => { + coreWindow.__kbnBundles__['plugin/foo'] = undefined; + + expect(() => { + read('foo'); + }).toThrowError(`Definition of plugin "foo" not found and may have failed to load.`); +}); + +it('handles plugin exports with a "plugin" export that is not a function', () => { + coreWindow.__kbnBundles__['plugin/foo'] = { + plugin: 1234, + } as any; + + expect(() => { + read('foo'); + }).toThrowError(`Definition of plugin "foo" should be a function.`); +}); + +it('returns the plugin initializer when the "plugin" named export is a function', () => { + const plugin: UnknownPluginInitializer = () => { + return undefined as any; + }; + + coreWindow.__kbnBundles__['plugin/foo'] = { plugin }; + + expect(read('foo')).toBe(plugin); +}); diff --git a/src/core/public/plugins/plugin_reader.ts b/src/core/public/plugins/plugin_reader.ts new file mode 100644 index 00000000000000..1907dfa6a3e99c --- /dev/null +++ b/src/core/public/plugins/plugin_reader.ts @@ -0,0 +1,52 @@ +/* + * 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 { PluginInitializer } from './plugin'; + +/** + * Unknown variant for internal use only for when plugins are not known. + * @internal + */ +export type UnknownPluginInitializer = PluginInitializer>; + +/** + * Custom window type for loading bundles. Do not extend global Window to avoid leaking these types. + * @internal + */ +export interface CoreWindow { + __kbnBundles__: { + [pluginBundleName: string]: { plugin: UnknownPluginInitializer } | undefined; + }; +} + +/** + * Reads the plugin's bundle declared in the global context. + */ +export function read(name: string) { + const coreWindow = (window as unknown) as CoreWindow; + const exportId = `plugin/${name}`; + const pluginExport = coreWindow.__kbnBundles__[exportId]; + if (!pluginExport) { + throw new Error(`Definition of plugin "${name}" not found and may have failed to load.`); + } else if (typeof pluginExport.plugin !== 'function') { + throw new Error(`Definition of plugin "${name}" should be a function.`); + } else { + return pluginExport.plugin; + } +} diff --git a/src/core/public/plugins/plugins_service.test.mocks.ts b/src/core/public/plugins/plugins_service.test.mocks.ts index a76078932518f2..85b84e561056ad 100644 --- a/src/core/public/plugins/plugins_service.test.mocks.ts +++ b/src/core/public/plugins/plugins_service.test.mocks.ts @@ -19,16 +19,16 @@ import { PluginName } from 'kibana/server'; import { Plugin } from './plugin'; -import { loadPluginBundleMock } from './plugin_loader.mock'; export type MockedPluginInitializer = jest.Mock>, any>; export const mockPluginInitializerProvider: jest.Mock< MockedPluginInitializer, [PluginName] -> = jest.fn().mockRejectedValue(new Error('No provider specified')); +> = jest.fn().mockImplementation(() => () => { + throw new Error('No provider specified'); +}); -export const mockLoadPluginBundle = loadPluginBundleMock.create(mockPluginInitializerProvider); -jest.mock('./plugin_loader', () => ({ - loadPluginBundle: mockLoadPluginBundle, +jest.mock('./plugin_reader', () => ({ + read: mockPluginInitializerProvider, })); diff --git a/src/core/public/plugins/plugins_service.test.ts b/src/core/public/plugins/plugins_service.test.ts index 688eaf4f2bfc7a..6d71844bc19c84 100644 --- a/src/core/public/plugins/plugins_service.test.ts +++ b/src/core/public/plugins/plugins_service.test.ts @@ -21,7 +21,6 @@ import { omit, pick } from 'lodash'; import { MockedPluginInitializer, - mockLoadPluginBundle, mockPluginInitializerProvider, } from './plugins_service.test.mocks'; @@ -32,6 +31,7 @@ import { PluginsServiceStartDeps, PluginsServiceSetupDeps, } from './plugins_service'; + import { InjectedPluginMetadata } from '../injected_metadata'; import { notificationServiceMock } from '../notifications/notifications_service.mock'; import { applicationServiceMock } from '../application/application_service.mock'; @@ -152,10 +152,6 @@ describe('PluginsService', () => { ] as unknown) as [[PluginName, any]]); }); - afterEach(() => { - mockLoadPluginBundle.mockClear(); - }); - describe('#getOpaqueIds()', () => { it('returns dependency tree of symbols', () => { const pluginsService = new PluginsService(mockCoreContext, plugins); @@ -174,15 +170,6 @@ describe('PluginsService', () => { }); describe('#setup()', () => { - it('fails if any bundle cannot be loaded', async () => { - mockLoadPluginBundle.mockRejectedValueOnce(new Error('Could not load bundle')); - - const pluginsService = new PluginsService(mockCoreContext, plugins); - await expect(pluginsService.setup(mockSetupDeps)).rejects.toThrowErrorMatchingInlineSnapshot( - `"Could not load bundle"` - ); - }); - it('fails if any plugin instance does not have a setup function', async () => { mockPluginInitializers.set('pluginA', (() => ({})) as any); const pluginsService = new PluginsService(mockCoreContext, plugins); @@ -191,25 +178,6 @@ describe('PluginsService', () => { ); }); - it('calls loadPluginBundles with http and plugins', async () => { - const pluginsService = new PluginsService(mockCoreContext, plugins); - await pluginsService.setup(mockSetupDeps); - - expect(mockLoadPluginBundle).toHaveBeenCalledTimes(3); - expect(mockLoadPluginBundle).toHaveBeenCalledWith( - mockSetupDeps.http.basePath.prepend, - 'pluginA' - ); - expect(mockLoadPluginBundle).toHaveBeenCalledWith( - mockSetupDeps.http.basePath.prepend, - 'pluginB' - ); - expect(mockLoadPluginBundle).toHaveBeenCalledWith( - mockSetupDeps.http.basePath.prepend, - 'pluginC' - ); - }); - it('initializes plugins with PluginInitializerContext', async () => { const pluginsService = new PluginsService(mockCoreContext, plugins); await pluginsService.setup(mockSetupDeps); @@ -302,7 +270,6 @@ describe('PluginsService', () => { const pluginsService = new PluginsService(mockCoreContext, plugins); const promise = pluginsService.setup(mockSetupDeps); - jest.runAllTimers(); // load plugin bundles await flushPromises(); jest.runAllTimers(); // setup plugins diff --git a/src/core/public/plugins/plugins_service.ts b/src/core/public/plugins/plugins_service.ts index e698af689036d7..862aa5043ad4b7 100644 --- a/src/core/public/plugins/plugins_service.ts +++ b/src/core/public/plugins/plugins_service.ts @@ -93,9 +93,6 @@ export class PluginsService implements CoreService { - // Load plugin bundles - await this.loadPluginBundles(deps.http.basePath.prepend); - // Setup each plugin with required and optional plugin contracts const contracts = new Map(); for (const [pluginName, plugin] of this.plugins.entries()) { @@ -167,9 +164,4 @@ export class PluginsService implements CoreService string) { - // Load all bundles in parallel - return Promise.all([...this.plugins.values()].map(plugin => plugin.load(addBasePath))); - } } diff --git a/src/core/server/rendering/views/template.tsx b/src/core/server/rendering/views/template.tsx index b38259a84cb9cc..73e119a5a97e73 100644 --- a/src/core/server/rendering/views/template.tsx +++ b/src/core/server/rendering/views/template.tsx @@ -104,6 +104,10 @@ export const Template: FunctionComponent = ({ + + {/* Inject stylesheets into the before scripts so that KP plugins with bundled styles will override them */} + + {createElement('kbn-csp', { diff --git a/src/legacy/server/kbn_server.d.ts b/src/legacy/server/kbn_server.d.ts index ee8b25a5de8f19..81628ff230c626 100644 --- a/src/legacy/server/kbn_server.d.ts +++ b/src/legacy/server/kbn_server.d.ts @@ -153,6 +153,7 @@ export default class KbnServer { public server: Server; public inject: Server['inject']; public pluginSpecs: any[]; + public uiBundles: any; constructor( settings: Record, diff --git a/src/legacy/ui/ui_render/bootstrap/template.js.hbs b/src/legacy/ui/ui_render/bootstrap/template.js.hbs index 8a71c6ccb15064..b60442690af3c3 100644 --- a/src/legacy/ui/ui_render/bootstrap/template.js.hbs +++ b/src/legacy/ui/ui_render/bootstrap/template.js.hbs @@ -29,6 +29,7 @@ if (window.__kbnStrictCsp__ && window.__kbnCspNotEnforced__) { document.body.innerHTML = err.outerHTML; } + var stylesheetTarget = document.querySelector('head meta[name="add-styles-here"]') function loadStyleSheet(url, cb) { var dom = document.createElement('link'); dom.rel = 'stylesheet'; @@ -36,9 +37,10 @@ if (window.__kbnStrictCsp__ && window.__kbnCspNotEnforced__) { dom.href = url; dom.addEventListener('error', failure); dom.addEventListener('load', cb); - document.head.appendChild(dom); + document.head.insertBefore(dom, stylesheetTarget); } + var scriptsTarget = document.querySelector('head meta[name="add-scripts-here"]') function loadScript(url, cb) { var dom = document.createElement('script'); {{!-- NOTE: async = false is used to trigger async-download/ordered-execution as outlined here: https://www.html5rocks.com/en/tutorials/speed/script-loading/ --}} @@ -46,7 +48,7 @@ if (window.__kbnStrictCsp__ && window.__kbnCspNotEnforced__) { dom.src = url; dom.addEventListener('error', failure); dom.addEventListener('load', cb); - document.head.appendChild(dom); + document.head.insertBefore(dom, scriptsTarget); } function load(urls, cb) { diff --git a/src/legacy/ui/ui_render/ui_render_mixin.js b/src/legacy/ui/ui_render/ui_render_mixin.js index 801eecf5b608bd..d42d6d556b37be 100644 --- a/src/legacy/ui/ui_render/ui_render_mixin.js +++ b/src/legacy/ui/ui_render/ui_render_mixin.js @@ -19,13 +19,15 @@ import { createHash } from 'crypto'; import Boom from 'boom'; -import { resolve } from 'path'; +import Path from 'path'; import { i18n } from '@kbn/i18n'; import * as UiSharedDeps from '@kbn/ui-shared-deps'; import { AppBootstrap } from './bootstrap'; import { getApmConfig } from '../apm'; import { DllCompiler } from '../../../optimize/dynamic_dll_plugin'; +const uniq = (...items) => Array.from(new Set(items)); + /** * @typedef {import('../../server/kbn_server').default} KbnServer * @typedef {import('../../server/kbn_server').ResponseToolkit} ResponseToolkit @@ -39,7 +41,7 @@ import { DllCompiler } from '../../../optimize/dynamic_dll_plugin'; */ export function uiRenderMixin(kbnServer, server, config) { // render all views from ./views - server.setupViews(resolve(__dirname, 'views')); + server.setupViews(Path.resolve(__dirname, 'views')); const translationsCache = { translations: null, hash: null }; server.route({ @@ -136,6 +138,15 @@ export function uiRenderMixin(kbnServer, server, config) { ]), ]; + const kpPluginIds = uniq( + // load these plugins first, they are "shared" and other bundles access their + // public/index exports without considering topographic sorting by plugin deps (for now) + 'kibanaUtils', + 'esUiShared', + 'kibanaReact', + ...kbnServer.newPlatform.__internals.uiPlugins.public.keys() + ); + const jsDependencyPaths = [ ...UiSharedDeps.jsDepFilenames.map( filename => `${regularBundlePath}/kbn-ui-shared-deps/${filename}` @@ -148,20 +159,22 @@ export function uiRenderMixin(kbnServer, server, config) { ...dllJsChunks, `${regularBundlePath}/commons.bundle.js`, ]), - `${regularBundlePath}/plugin/kibanaUtils/kibanaUtils.plugin.js`, - `${regularBundlePath}/plugin/esUiShared/esUiShared.plugin.js`, - `${regularBundlePath}/plugin/kibanaReact/kibanaReact.plugin.js`, - ]; - const uiPluginIds = [...kbnServer.newPlatform.__internals.uiPlugins.public.keys()]; + ...kpPluginIds.map( + pluginId => `${regularBundlePath}/plugin/${pluginId}/${pluginId}.plugin.js` + ), + ]; // These paths should align with the bundle routes configured in - // src/optimize/bundles_route/bundles_route.js + // src/optimize/bundles_route/bundles_route.ts const publicPathMap = JSON.stringify({ core: `${regularBundlePath}/core/`, 'kbn-ui-shared-deps': `${regularBundlePath}/kbn-ui-shared-deps/`, - ...uiPluginIds.reduce( - (acc, pluginId) => ({ ...acc, [pluginId]: `${regularBundlePath}/plugin/${pluginId}/` }), + ...kpPluginIds.reduce( + (acc, pluginId) => ({ + ...acc, + [pluginId]: `${regularBundlePath}/plugin/${pluginId}/`, + }), {} ), });