Skip to content

Commit

Permalink
[uiExports] migrate uiApp "uses" to explicit imports in apps (elastic…
Browse files Browse the repository at this point in the history
…#17828)

* [uiExports] migrate uiApp "uses" to explicit imports in apps

* [uiApp] update tests for getModules() method

* [optimize/uiExports] improve naming and comments

* [uiExports] sort imports so they load in the same order as before

* [testHarness] load hacks when testing in the browser

* [x-pack/uiExports] use new uiExports modules

* [testHarness] describe why we import uiExports/hacks

* [optimize] remove needless [].concat()

* [optimize/createUiExportsModule] string.includes > string.indexOf

* [uiExports/createUiExportsModule] remove needless capture of module exports

(cherry picked from commit e1a2fcb)
  • Loading branch information
spalger committed May 3, 2018
1 parent a26ddaf commit a6c0c62
Show file tree
Hide file tree
Showing 22 changed files with 125 additions and 111 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ module.exports = {
// instructs import/no-extraneous-dependencies to treat modules
// in plugins/ or ui/ namespace as "core modules" so they don't
// trigger failures for not being listed in package.json
'import/core-modules': ['plugins', 'ui'],
'import/core-modules': ['plugins', 'ui', 'uiExports'],

'import/resolver': {
'@kbn/eslint-import-resolver-kibana': {
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@
"ui-select": "0.19.6",
"url-loader": "0.5.9",
"uuid": "3.0.1",
"val-loader": "^1.1.0",
"validate-npm-package-name": "2.2.2",
"vega-lib": "^3.3.1",
"vega-lite": "^2.4.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ function initContext(file, config) {

exports.resolve = function resolveKibanaPath(importRequest, file, config) {
config = config || {};

// these modules are simulated by webpack, so there is no
// path to resolve to and no reason to do any more work
if (importRequest.startsWith('uiExports/')) {
return {
found: true,
path: null,
};
}

const { webpackConfig, aliasEntries } = initContext(file, config);
let isPathRequest = getIsPathRequest(importRequest);

Expand Down
16 changes: 0 additions & 16 deletions src/core_plugins/kibana/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,6 @@ export default function (kibana) {
listed: false,
description: 'the kibana you know and love',
main: 'plugins/kibana/kibana',
uses: [
'home',
'visTypes',
'visResponseHandlers',
'visRequestHandlers',
'visEditorTypes',
'savedObjectTypes',
'spyModes',
'fieldFormats',
'fieldFormatEditors',
'navbarExtensions',
'managementSections',
'devTools',
'docViews',
'embeddableFactories',
],
},

links: [
Expand Down
16 changes: 16 additions & 0 deletions src/core_plugins/kibana/public/kibana.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,22 @@ import chrome from 'ui/chrome';
import routes from 'ui/routes';
import { uiModules } from 'ui/modules';

// import the uiExports that we want to "use"
import 'uiExports/home';
import 'uiExports/visTypes';
import 'uiExports/visResponseHandlers';
import 'uiExports/visRequestHandlers';
import 'uiExports/visEditorTypes';
import 'uiExports/savedObjectTypes';
import 'uiExports/spyModes';
import 'uiExports/fieldFormats';
import 'uiExports/fieldFormatEditors';
import 'uiExports/navbarExtensions';
import 'uiExports/managementSections';
import 'uiExports/devTools';
import 'uiExports/docViews';
import 'uiExports/embeddableFactories';

import 'ui/autoload/all';
import './home';
import './discover';
Expand Down
4 changes: 0 additions & 4 deletions src/core_plugins/timelion/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ export default function (kibana) {
description: 'Time series expressions for everything',
icon: 'plugins/timelion/icon.svg',
main: 'plugins/timelion/app',
uses: [
'fieldFormats',
'savedObjectTypes'
]
},
hacks: [
'plugins/timelion/lib/panel_registry',
Expand Down
4 changes: 4 additions & 0 deletions src/core_plugins/timelion/public/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import { notify, fatalError, toastNotifications } from 'ui/notify';
import { timezoneProvider } from 'ui/vis/lib/timezone';
import { recentlyAccessed } from 'ui/persisted_log';

// import the uiExports that we want to "use"
import 'uiExports/fieldFormats';
import 'uiExports/savedObjectTypes';

require('ui/autoload/all');
require('plugins/timelion/directives/cells/cells');
require('plugins/timelion/directives/fixed_element');
Expand Down
24 changes: 24 additions & 0 deletions src/optimize/base_optimizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,30 @@ export default class BaseOptimizer {

new webpack.NoEmitOnErrorsPlugin(),

// replace imports for `uiExports/*` modules with a synthetic module
// created by create_ui_exports_module.js
new webpack.NormalModuleReplacementPlugin(/^uiExports\//, (resource) => {
// the map of uiExport types to module ids
const extensions = this.uiBundles.getAppExtensions();

// everything following the first / in the request is
// treated as a type of appExtension
const type = resource.request.slice(resource.request.indexOf('/') + 1);

resource.request = [
// the "val-loader" is used to execute create_ui_exports_module
// and use its return value as the source for the module in the
// bundle. This allows us to bypass writing to the file system
require.resolve('val-loader'),
'!',
require.resolve('./create_ui_exports_module'),
'?',
// this JSON is parsed by create_ui_exports_module and determines
// what require() calls it will execute within the bundle
JSON.stringify({ type, modules: extensions[type] || [] })
].join('');
}),

...this.uiBundles.getWebpackPluginProviders()
.map(provider => provider(webpack)),
],
Expand Down
21 changes: 21 additions & 0 deletions src/optimize/create_ui_exports_module.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// We normalize all path separators to `/` in generated files
function normalizePath(path) {
return path.replace(/[\\\/]+/g, '/');
}

export default function () {
if (!module.id.includes('?')) {
throw new Error('create_ui_exports_module loaded without JSON args in module.id');
}

const { type, modules } = JSON.parse(module.id.slice(module.id.indexOf('?') + 1));
const comment = `// dynamically generated to load ${type} uiExports from plugins`;
const requires = modules
.sort((a, b) => a.localeCompare(b))
.map(m => `require('${normalizePath(m)}')`)
.join('\n ');

return {
code: `${comment}\n${requires}\n`
};
}
6 changes: 6 additions & 0 deletions src/ui/public/chrome/chrome.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ translationsApi(chrome, internals);

const waitForBootstrap = new Promise(resolve => {
chrome.bootstrap = function () {
// 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
require('uiExports/chromeNavControls');
require('uiExports/hacks');

chrome.setupAngular();
angular.bootstrap(document.body, ['kibana']);
resolve();
Expand Down
3 changes: 3 additions & 0 deletions src/ui/public/test_harness/test_harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,8 @@ afterEach(function () {

// Kick off mocha, called at the end of test entry files
export function bootstrap() {
// load the hacks since we aren't actually bootstrapping the
// chrome, which is where the hacks would normally be loaded
require('uiExports/hacks');
chrome.setupAngular();
}
45 changes: 0 additions & 45 deletions src/ui/ui_apps/__tests__/ui_app.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import sinon from 'sinon';
import expect from 'expect.js';
import Chance from 'chance';

import { UiApp } from '../ui_app';
import { UiNavLink } from '../../ui_nav_links';

const chance = new Chance();

function createStubUiAppSpec(extraParams) {
return {
id: 'uiapp-test',
Expand All @@ -18,25 +15,13 @@ function createStubUiAppSpec(extraParams) {
linkToLastSubUrl: true,
hidden: false,
listed: false,
uses: [
'visTypes',
'chromeNavControls',
'hacks',
],
...extraParams
};
}

function createStubKbnServer() {
return {
plugins: [],
uiExports: {
appExtensions: {
hacks: [
'plugins/foo/hack'
]
}
},
config: {
get: sinon.stub()
.withArgs('server.basePath')
Expand Down Expand Up @@ -129,7 +114,6 @@ describe('ui apps / UiApp', () => {
it('includes main and hack modules', () => {
expect(app.getModules()).to.eql([
'main.js',
'plugins/foo/hack'
]);
});

Expand Down Expand Up @@ -305,34 +289,5 @@ describe('ui apps / UiApp', () => {
const app = createUiApp({ id: 'foo', main: 'bar' });
expect(app.getModules()).to.eql(['bar']);
});

it('returns appExtensions for used types only, in alphabetical order, starting with main module', () => {
const kbnServer = createStubKbnServer();
kbnServer.uiExports.appExtensions = {
abc: chance.shuffle([
'a',
'b',
'c',
]),
def: chance.shuffle([
'd',
'e',
'f',
])
};

const appExtensionType = chance.shuffle(Object.keys(kbnServer.uiExports.appExtensions))[0];
const appSpec = {
id: 'foo',
main: 'bar',
uses: [appExtensionType],
};

const app = createUiApp(appSpec, kbnServer);
expect(app.getModules()).to.eql([
'bar',
...appExtensionType.split(''),
]);
});
});
});
15 changes: 1 addition & 14 deletions src/ui/ui_apps/ui_app.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export class UiApp {
linkToLastSubUrl,
listed,
url = `/app/${id}`,
uses = []
} = spec;

if (!id) {
Expand All @@ -38,18 +37,6 @@ export class UiApp {
throw new Error(`Unknown plugin id "${this._pluginId}"`);
}

const { appExtensions = [] } = kbnServer.uiExports;
this._modules = [].concat(
this._main || [],
uses
// flatten appExtensions for used types
.reduce((acc, type) => acc.concat(appExtensions[type] || []), [])
// de-dupe app extension module ids
.reduce((acc, item) => !item || acc.includes(item) ? acc : acc.concat(item), [])
// sort app extension module ids alphabetically
.sort((a, b) => a.localeCompare(b))
);

if (!this.isHidden()) {
// unless an app is hidden it gets a navlink, but we only respond to `getNavLink()`
// if the app is also listed. This means that all apps in the kibanaPayload will
Expand Down Expand Up @@ -93,7 +80,7 @@ export class UiApp {
}

getModules() {
return this._modules;
return this._main ? [this._main] : [];
}

_getPlugin() {
Expand Down
6 changes: 6 additions & 0 deletions src/ui/ui_bundles/ui_bundles_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export class UiBundlesController {
matchBase: true
});

this._appExtensions = uiExports.appExtensions || {};

this._webpackAliases = {
...getWebpackAliases(pluginSpecs),
...uiExports.webpackAliases
Expand Down Expand Up @@ -103,6 +105,10 @@ export class UiBundlesController {
return this._webpackAliases;
}

getAppExtensions() {
return this._appExtensions;
}

isDevMode() {
return this._env === 'development';
}
Expand Down
14 changes: 6 additions & 8 deletions src/ui/ui_exports/ui_export_types/ui_apps.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { uniq } from 'lodash';

import { flatConcatAtType } from './reduce';
import { alias, mapSpec, wrap } from './modify_reduce';

Expand All @@ -16,13 +14,18 @@ function applySpecDefaults(spec, type, pluginSpec) {
linkToLastSubUrl = true,
listed = !hidden,
url = `/app/${id}`,
uses = [],
} = spec;

if (spec.injectVars) {
throw new Error(`[plugin:${pluginId}] uiExports.app.injectVars has been removed. Use server.injectUiAppVars('${id}', () => { ... })`);
}

if (spec.uses) {
throw new Error(
`[plugin:${pluginId}] uiExports.app.uses has been removed. Import these uiExport types with "import 'uiExports/{type}'"`
);
}

return {
pluginId,
id,
Expand All @@ -35,11 +38,6 @@ function applySpecDefaults(spec, type, pluginSpec) {
linkToLastSubUrl,
listed,
url,
uses: uniq([
...uses,
'chromeNavControls',
'hacks',
]),
};
}

Expand Down
12 changes: 0 additions & 12 deletions x-pack/plugins/dashboard_mode/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,6 @@ export function dashboardMode(kibana) {
hidden: true,
description: 'view dashboards',
main: 'plugins/dashboard_mode/dashboard_viewer',
uses: [
'visTypes',
'visResponseHandlers',
'visRequestHandlers',
'visEditorTypes',
'savedObjectTypes',
'embeddableFactories',
'spyModes',
'navbarExtensions',
'docViews',
'fieldFormats'
],
links: [
{
id: 'kibana:dashboard',
Expand Down
12 changes: 12 additions & 0 deletions x-pack/plugins/dashboard_mode/public/dashboard_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,18 @@ import chrome from 'ui/chrome';
import routes from 'ui/routes';
import { uiModules } from 'ui/modules';

// import the uiExports that we want to "use"
import 'uiExports/visTypes';
import 'uiExports/visResponseHandlers';
import 'uiExports/visRequestHandlers';
import 'uiExports/visEditorTypes';
import 'uiExports/savedObjectTypes';
import 'uiExports/embeddableFactories';
import 'uiExports/spyModes';
import 'uiExports/navbarExtensions';
import 'uiExports/docViews';
import 'uiExports/fieldFormats';

import _ from 'lodash';
import 'ui/autoload/all';
import 'plugins/kibana/dashboard';
Expand Down
Loading

0 comments on commit a6c0c62

Please sign in to comment.