Skip to content

Commit

Permalink
Improve dll plugin relation with webpackshims (#30129) (#31766)
Browse files Browse the repository at this point in the history
* chore(NA): remove specific watch for x-pack webpackShims folder.

* chore(NA): remove xpack security plugin angular-ui-select webpackShim.

* chore(NA): bump ui-select version on x-pack to match the one used on oss kibana

* chore(NA): remove manual searching for webpackShim imports into the dll plugin. chore(NA): explicit avoid max dll compilations in all environments for the dll plugin. chore(NA): explicit throw an error and list all the not allowed modules bundled into the dll bundle.

* refact(NA): move ui related actions inside webpackShims to proper ui related files

* chore(NA): move angular ui dependencies from webpackShims to kibana core module.

* test(NA): enable xpack jest tests to be able to resolve plugins/xpack_main/*. refact(NA): rewrite code for the old xpack jquery flot webpackShim.

* refact(NA): use the already declared ui module get to list the dependencies for the kibana legacy core plugin.

* chore(NA): move angular ui requires to a better centralized place.

* refact(NA): rename areMaxCompilationsPerformed to assertMaxCompilations.

* refact(NA): remove unnecessary promise resolve on async function.

* refact(NA): remove unnecessary promise resolve on async function.

* refact(NA): apply changes according pr review.

* refact(NA): change from requires to imports in xpack_main plugin jquery flots.

* refact(NA): jquery flots missing statements.

* fix(na): linting problems.

* chore(na): re add jquery flot requires instead of imports.

* refact(NA): moving jquery flots from require to import. test(NA): fix mock for jquery_flot.

* feat(na): allow dynamic dll plugin public modules on dll bundle.

* feat(NA): step verification to not allow modules from xpack source.

* chore(NA): fix linting problems.
  • Loading branch information
mistic authored Feb 22, 2019
1 parent 9118b13 commit 963b26b
Show file tree
Hide file tree
Showing 46 changed files with 220 additions and 261 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,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', 'uiExports'],
'import/core-modules': ['plugins', 'legacy/ui', 'uiExports'],

'import/resolver': {
'@kbn/eslint-import-resolver-kibana': {
Expand Down
1 change: 0 additions & 1 deletion src/cli/cluster/cluster_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ export default class ClusterManager {
fromRoot('x-pack/common'),
fromRoot('x-pack/plugins'),
fromRoot('x-pack/server'),
fromRoot('x-pack/webpackShims'),
fromRoot('config'),
...extraPaths,
].map(path => resolve(path));
Expand Down
3 changes: 1 addition & 2 deletions src/dev/precommit_hook/casing_check_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,14 @@ export const TEMPORARILY_IGNORED_PATHS = [
'packages/kbn-ui-framework/doc_site/src/images/elastic-logo.svg',
'packages/kbn-ui-framework/doc_site/src/images/hint-arrow.svg',
'packages/kbn-ui-framework/doc_site/src/images/react-logo.svg',
'webpackShims/angular-ui-select.js',
'webpackShims/elasticsearch-browser.js',
'webpackShims/moment-timezone.js',
'webpackShims/ui-bootstrap.js',
'x-pack/plugins/graph/public/graphClientWorkspace.js',
'x-pack/plugins/graph/public/angular-venn-simple.js',
'x-pack/plugins/index_management/public/lib/editSettings.js',
'x-pack/plugins/license_management/public/store/reducers/licenseManagement.js',
'x-pack/plugins/monitoring/public/components/sparkline/__mocks__/jquery-flot.js',
'x-pack/plugins/monitoring/public/components/sparkline/__mocks__/plugins/xpack_main/jquery_flot.js',
'x-pack/plugins/ml/public/jobs/new_job/simple/components/watcher/email-influencers.html',
'x-pack/plugins/monitoring/public/icons/alert-blue.svg',
'x-pack/plugins/monitoring/public/icons/health-gray.svg',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import 'ui/directives/css_truncate';
import 'ui/directives/field_name';
import 'ui/filters/unique';
import './discover_field';
import 'angular-ui-select';
import 'ui/angular_ui_select';
import _ from 'lodash';
import $ from 'jquery';
import rison from 'rison-node';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import _ from 'lodash';
import angular from 'angular';
import 'angular-elastic/elastic';
import rison from 'rison-node';
import { savedObjectManagementRegistry } from '../../saved_object_registry';
import objectViewHTML from './_view.html';
Expand All @@ -40,7 +41,7 @@ uiRoutes
k7Breadcrumbs: getViewBreadcrumbs
});

uiModules.get('apps/management')
uiModules.get('apps/management', ['monospaced.elastic'])
.directive('kbnManagementObjectsView', function (kbnIndex, confirmModal, i18n) {
return {
restrict: 'E',
Expand Down
8 changes: 8 additions & 0 deletions src/legacy/ui/public/angular-bootstrap/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
* TODO: Write custom components that address our needs to directly and deprecate these Bootstrap components.
*/

import 'angular';

import { uiModules } from 'ui/modules';

uiModules.get('kibana', [
'ui.bootstrap',
]);

/*
* angular-ui-bootstrap
* http://angular-ui.github.io/bootstrap/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@
* under the License.
*/

require('jquery');
require('angular');
require('angular-sanitize');
require('ui-select/dist/select');
require('ui-select/dist/select.css');
import 'jquery';
import 'angular';
import 'angular-sanitize';
import 'ui-select/dist/select';

require('ui/modules').get('kibana', ['ui.select', 'ngSanitize']);
import { uiModules } from 'ui/modules';

uiModules.get('kibana', ['ui.select', 'ngSanitize']);
4 changes: 4 additions & 0 deletions src/legacy/ui/public/autoload/modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,7 @@ import '../saved_objects/ui/saved_object_save_as_checkbox';
import '../react_components';
import '../i18n';
import '../query_bar/directive';

import '@elastic/ui-ace';
import { uiModules } from 'ui/modules';
uiModules.get('kibana', ['ui.ace']);
2 changes: 1 addition & 1 deletion src/legacy/ui/public/styles/bootstrap/buttons.less
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* angular-ui-select depends upon these styles. Don't use them in your markup.
* ui/angular-ui-select depends upon these styles. Don't use them in your markup.
* Please use the UI Framework styles instead.
*/

Expand Down
4 changes: 4 additions & 0 deletions src/legacy/ui/public/test_harness/test_harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/

// chrome expects to be loaded first, let it get its way
import $ from 'jquery';
import bindJqueryToFindTestSubject from 'ui/jquery/find_test_subject';
import chrome from '../chrome';

import { parse as parseUrl } from 'url';
Expand All @@ -30,6 +32,8 @@ import './test_harness.css';
import 'ng_mock';
import { setupTestSharding } from './test_sharding';

bindJqueryToFindTestSubject($);

const { query } = parseUrl(window.location.href, true);
if (query && query.mocha) {
try {
Expand Down
6 changes: 5 additions & 1 deletion src/legacy/ui/public/tooltip/tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,18 @@
* under the License.
*/

import 'ui/angular-bootstrap';
import html from './tooltip.html';
import chrome from 'ui/chrome';
import { uiModules } from 'ui/modules';

require('ui-bootstrap')
uiModules.get('kibana')
.config(function ($tooltipProvider) {
// we use the uiSettings client because the config service is not available in the config phase
const uiSettings = chrome.getUiSettingsClient();

$tooltipProvider.setTriggers({ 'mouseenter': 'mouseleave click' });

$tooltipProvider.options({
placement: 'bottom',
animation: !uiSettings.get('accessibility:disableAnimations'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,21 @@
* under the License.
*/

require('angular');
require('ui/angular-bootstrap');
var uiModules = require('ui/modules').uiModules;
import path from 'path';

var kibana = uiModules.get('kibana', ['ui.bootstrap']);
export function notInNodeModules(checkPath) {
return !checkPath.includes(`${path.sep}node_modules${path.sep}`);
}

module.exports = kibana.config(function ($tooltipProvider) {
$tooltipProvider.setTriggers({ 'mouseenter': 'mouseleave click' });
});
export function notInNodeModulesOrWebpackShims(checkPath) {
return notInNodeModules(checkPath)
&& !checkPath.includes(`${path.sep}webpackShims${path.sep}`);
}

export function inPluginNodeModules(checkPath) {
return checkPath.match(/[\/\\]plugins.*[\/\\]node_modules/);
}

export function inDllPluginPublic(checkPath) {
return checkPath.includes(`${path.sep}dynamic_dll_plugin/public${path.sep}`);
}
105 changes: 92 additions & 13 deletions src/optimize/dynamic_dll_plugin/dll_compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,21 @@
*/

import { configModel } from './dll_config_model';
import { notInNodeModulesOrWebpackShims, notInNodeModules, inDllPluginPublic } from './dll_allowed_modules';
import { fromRoot } from '../../legacy/utils';
import { PUBLIC_PATH_PLACEHOLDER } from '../public_path_placeholder';
import fs from 'fs';
import mkdirp from 'mkdirp';
import webpack from 'webpack';
import { promisify } from 'util';
import path from 'path';
import rimraf from 'rimraf';

const readFileAsync = promisify(fs.readFile);
const mkdirpAsync = promisify(mkdirp);
const existsAsync = promisify(fs.exists);
const writeFileAsync = promisify(fs.writeFile);
const rimrafAsync = promisify(rimraf);

export class DllCompiler {
static getRawDllConfig(uiBundles = {}, babelLoaderCacheDir = '', threadLoaderPoolConfig = {}) {
Expand Down Expand Up @@ -163,7 +166,26 @@ export class DllCompiler {
async run(dllEntries) {
const dllConfig = this.dllConfigGenerator(this.rawDllConfig);
await this.upsertEntryFile(dllEntries);
await this.runWebpack(dllConfig());

try {
this.logWithMetadata(['info', 'optimize:dynamic_dll_plugin'], 'Client vendors dll compilation started');

await this.runWebpack(dllConfig());

this.logWithMetadata(
['info', 'optimize:dynamic_dll_plugin'],
`Client vendors dll compilation finished with success`
);
} catch (e) {
this.logWithMetadata(
['fatal', 'optimize:dynamic_dll_plugin'],
`Client vendors dll compilation failed`
);

// Still throw the original error has here we just want
// log the fail message
throw e;
}

// Style dll file isn't always created but we are
// expecting it to exist always as we are referencing
Expand All @@ -182,9 +204,7 @@ export class DllCompiler {

async runWebpack(config) {
return new Promise((resolve, reject) => {
this.logWithMetadata(['info', 'optimize:dynamic_dll_plugin'], 'Client vendors dll compilation started');

webpack(config, (err, stats) => {
webpack(config, async (err, stats) => {
// If a critical error occurs or we have
// errors in the stats compilation,
// reject the promise and logs the errors
Expand All @@ -197,18 +217,77 @@ export class DllCompiler {
}));

if (webpackErrors) {
this.logWithMetadata(
['fatal', 'optimize:dynamic_dll_plugin'],
`Client vendors dll compilation failed`
);
// Reject with webpack fatal errors
return reject(webpackErrors);
}

// Otherwise let it proceed
this.logWithMetadata(
['info', 'optimize:dynamic_dll_plugin'],
`Client vendors dll compilation finished with success`
);
// Identify if we have not allowed modules
// bundled inside the dll bundle
const notAllowedModules = [];

stats.compilation.modules.forEach((module) => {
// ignore if no module or userRequest are defined
if (!module || !module.resource) {
return;
}

// ignore if this module represents the
// dll entry file
if (module.resource === this.getEntryPath()) {
return;
}

// ignore if this module is part of the
// files inside dynamic dll plugin public folder
if (inDllPluginPublic(module.resource)) {
return;
}

// A module is not allowed if it's not a node_module, a webpackShim
// or the reasons from being bundled into the dll are not node_modules
if(notInNodeModulesOrWebpackShims(module.resource)) {
const reasons = module.reasons || [];

reasons.forEach((reason) => {
// Skip if we can't read the reason info
if (!reason || !reason.module || !reason.module.resource) {
return;
}

// Is the reason for this module being bundle a
// node_module or no?
if (notInNodeModules(reason.module.resource)) {
notAllowedModules.push(module.resource);
return;
}

// Even when the reason for the module comes from
// node_modules directory, assure it's not from
// node_modules/x-pack source code but from a real node_module
const dirs = reason.module.resource.split(path.sep);
const nodeModuleName = dirs[dirs.lastIndexOf('node_modules') + 1];
const inXpackSource = nodeModuleName === 'x-pack';

if (inXpackSource) {
notAllowedModules.push(module.resource);
}
});
}
});

if (notAllowedModules.length) {
// Delete the built dll, as it contains invalid modules, and reject listing
// all the not allowed modules
try {
await rimrafAsync(this.rawDllConfig.outputPath);
} catch (e) {
return reject(e);
}

return reject(`The following modules are not allowed to be bundled into the dll: \n${notAllowedModules.join('\n')}`);
}

// Otherwise it has succeed
return resolve(stats);
});
});
Expand Down
4 changes: 2 additions & 2 deletions src/optimize/dynamic_dll_plugin/dll_entry_template.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@

export function dllEntryTemplate(requirePaths = []) {
return [
`require('dll/set_csp_nonce')`,
`require('dll/set_csp_nonce');`,
...requirePaths
.map(path => `require('${path}')`)
.map(path => `require('${path}');`)
.sort()
].join('\n');
}
Loading

0 comments on commit 963b26b

Please sign in to comment.