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

Enables tree shaking in production for plugins #62390

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,10 @@
"xmlbuilder": "13.0.2",
"zlib": "^1.0.5"
},
"sideEffects": [
"./webpackShims/*.js",
"./src/plugins/console/public/application/models/legacy_core_editor/mode/*.{js,ts}"
],
"engines": {
"node": "10.19.0",
"yarn": "^1.21.1"
Expand Down
2 changes: 0 additions & 2 deletions packages/kbn-babel-preset/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
"@babel/plugin-proposal-class-properties": "^7.8.3",
"@babel/plugin-proposal-nullish-coalescing-operator": "^7.8.3",
"@babel/plugin-proposal-optional-chaining": "^7.9.0",
"@babel/plugin-syntax-dynamic-import": "^7.8.3",
"@babel/plugin-transform-modules-commonjs": "^7.9.0",
"@babel/preset-env": "^7.9.0",
"@babel/preset-react": "^7.9.1",
"@babel/preset-typescript": "^7.9.0",
Expand Down
2 changes: 0 additions & 2 deletions packages/kbn-babel-preset/webpack_preset.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ module.exports = () => {
require('./common_preset'),
],
plugins: [
require.resolve('@babel/plugin-transform-modules-commonjs'),
require.resolve('@babel/plugin-syntax-dynamic-import'),
[
require.resolve('babel-plugin-styled-components'),
{
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-i18n/babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ module.exports = {
env: {
web: {
presets: ['@kbn/babel-preset/webpack_preset'],
plugins: [require.resolve('@babel/plugin-transform-modules-commonjs')],
tylersmalley marked this conversation as resolved.
Show resolved Hide resolved
},
node: {
presets: ['@kbn/babel-preset/node_preset'],
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-i18n/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"devDependencies": {
"@babel/cli": "^7.8.4",
"@babel/core": "^7.9.0",
"@babel/plugin-transform-modules-commonjs": "^7.9.0",
"@kbn/babel-preset": "1.0.0",
"@kbn/dev-utils": "1.0.0",
"@types/intl-relativeformat": "^2.1.0",
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-interpreter/.babelrc
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{
"presets": ["@kbn/babel-preset/webpack_preset"],
"plugins": [
"@babel/plugin-transform-modules-commonjs",
"@babel/plugin-syntax-dynamic-import",
["@babel/plugin-transform-runtime", {
"regenerator": true
}]
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-interpreter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
"devDependencies": {
"@babel/cli": "^7.8.4",
"@babel/core": "^7.9.0",
"@babel/plugin-syntax-dynamic-import": "^7.8.3",
"@babel/plugin-transform-modules-commonjs": "^7.9.0",
"@babel/plugin-transform-runtime": "^7.9.0",
"@kbn/babel-preset": "1.0.0",
"@kbn/dev-utils": "1.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,10 @@ describe('js support', () => {
it('transpiles js file', () => {
const transformer = createServerCodeTransformer();
expect(transformer(JS_FIXTURE, JS_FIXTURE_PATH)).toMatchInlineSnapshot(`
"\\"use strict\\";

var _util = _interopRequireDefault(require(\\"util\\"));

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

/* eslint-disable */
console.log(_util.default.format('hello world'));"
`);
"/* eslint-disable */
Copy link
Contributor Author

@tylersmalley tylersmalley May 5, 2020

Choose a reason for hiding this comment

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

This whole file is removed in #65332

import util from 'util';
console.log(util.format('hello world'));"
`);
});

it('throws errors for js syntax errors', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

export * from './lib';
export { fooLibFn } from './lib';
export * from './ext';

export async function getFoo() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,7 @@
export function fooLibFn() {
return 'foo';
}

export function unused() {
return 'UNUSED_FUNCTION';
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ it('builds expected bundles, saves bundle counts to metadata', async () => {
repoRoot: MOCK_REPO_DIR,
pluginScanDirs: [Path.resolve(MOCK_REPO_DIR, 'plugins')],
maxWorkerCount: 1,
dist: true,
dist: false,
});

expect(config).toMatchSnapshot('OptimizerConfig');
Expand Down Expand Up @@ -125,12 +125,7 @@ it('builds expected bundles, saves bundle counts to metadata', async () => {
);
assert('produce zero unexpected states', otherStates.length === 0, otherStates);

expectFileMatchesSnapshotWithCompression('plugins/foo/target/public/foo.plugin.js', 'foo bundle');
expectFileMatchesSnapshotWithCompression(
'plugins/foo/target/public/1.plugin.js',
'1 async bundle'
);
expectFileMatchesSnapshotWithCompression('plugins/bar/target/public/bar.plugin.js', 'bar bundle');
expectFile('plugins/foo/target/public/foo.plugin.js').toContain('UNUSED_FUNCTION');

const foo = config.bundles.find(b => b.id === 'foo')!;
expect(foo).toBeTruthy();
Expand Down Expand Up @@ -195,11 +190,86 @@ it('uses cache on second run and exist cleanly', async () => {
"initializing",
"initializing",
"initialized",
"initialized",
"running",
"running",
"running",
"success",
]
`);
});

it('prepares assets for distribution', async () => {
const config = OptimizerConfig.create({
repoRoot: MOCK_REPO_DIR,
pluginScanDirs: [Path.resolve(MOCK_REPO_DIR, 'plugins')],
maxWorkerCount: 1,
dist: true,
});

expect(config).toMatchSnapshot('OptimizerConfig');

const log = new ToolingLog({
level: 'error',
writeTo: {
write(chunk) {
if (chunk.endsWith('\n')) {
chunk = chunk.slice(0, -1);
}
// eslint-disable-next-line no-console
console.error(chunk);
},
},
});

await runOptimizer(config)
.pipe(logOptimizerState(log, config), toArray())
.toPromise();

expectFileMatchesSnapshotWithCompression('plugins/foo/target/public/foo.plugin.js', 'foo bundle');
expectFileMatchesSnapshotWithCompression(
'plugins/foo/target/public/1.plugin.js',
'1 async bundle'
);
expectFileMatchesSnapshotWithCompression('plugins/bar/target/public/bar.plugin.js', 'bar bundle');

// removed during tree shaking
expectFile('plugins/foo/target/public/foo.plugin.js').not.toContain('UNUSED_FUNCTION');

const foo = config.bundles.find(b => b.id === 'foo')!;
expect(foo).toBeTruthy();
foo.cache.refresh();
expect(foo.cache.getModuleCount()).toBe(1);
expect(foo.cache.getReferencedFiles()).toMatchInlineSnapshot(`
Array [
<absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/foo/public/async_import.ts,
]
`);

const bar = config.bundles.find(b => b.id === 'bar')!;
expect(bar).toBeTruthy();
bar.cache.refresh();
expect(bar.cache.getModuleCount()).toBe(10);

expect(bar.cache.getReferencedFiles()).toMatchInlineSnapshot(`
Array [
<absolute path>/node_modules/css-loader/package.json,
<absolute path>/node_modules/style-loader/package.json,
<absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/bar/public/legacy/styles.scss,
<absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/foo/public/async_import.ts,
<absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/src/legacy/ui/public/icon.svg,
]
`);
});

/**
* Helper to provide expect for a file content
*/
const expectFile = (filePath: string) => {
const raw = Fs.readFileSync(Path.resolve(MOCK_REPO_DIR, filePath), 'utf8');
return expect(raw);
};

/**
* Verifies that the file matches the expected output and has matching compressed variants.
*/
Expand Down
31 changes: 18 additions & 13 deletions packages/kbn-optimizer/src/worker/webpack.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export function getWebpackConfig(bundle: Bundle, worker: WorkerConfig) {
options: {
key: bundle.id,
},
sideEffects: true,
},
{
test: /\.css$/,
Expand All @@ -164,6 +165,7 @@ export function getWebpackConfig(bundle: Bundle, worker: WorkerConfig) {
},
},
],
sideEffects: true,
},
{
test: /\.scss$/,
Expand Down Expand Up @@ -261,6 +263,7 @@ export function getWebpackConfig(bundle: Bundle, worker: WorkerConfig) {
loader: require.resolve('./theme_loader'),
},
],
sideEffects: true,
},
{
test: /\.(woff|woff2|ttf|eot|svg|ico|png|jpg|gif|jpeg)(\?|$)/,
Expand Down Expand Up @@ -320,6 +323,16 @@ export function getWebpackConfig(bundle: Bundle, worker: WorkerConfig) {
IS_KIBANA_DISTRIBUTABLE: `"true"`,
},
}),
new TerserPlugin({
cache: false,
sourceMap: false,
extractComments: false,
parallel: false,
terserOptions: {
compress: true,
mangle: false,
},
}),
new CompressionPlugin({
algorithm: 'brotliCompress',
filename: '[path].br',
Expand All @@ -331,20 +344,12 @@ export function getWebpackConfig(bundle: Bundle, worker: WorkerConfig) {
test: /\.(js|css)$/,
}),
],

optimization: {
minimizer: [
new TerserPlugin({
cache: false,
sourceMap: false,
extractComments: false,
parallel: false,
terserOptions: {
compress: false,
mangle: false,
},
}),
],
usedExports: true,

// NODE: this is the default with `mode: production` and results in
// less than ideal module name reporting
concatenateModules: true,
},
};

Expand Down
13 changes: 11 additions & 2 deletions src/optimize/base_optimizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ const STATS_WARNINGS_FILTER = new RegExp(
);
const IS_CODE_COVERAGE = !!process.env.CODE_COVERAGE;

const LEGACY_PRESETS = {
plugins: [
require.resolve('@babel/plugin-transform-modules-commonjs'),
require.resolve('@babel/plugin-syntax-dynamic-import'),
],
};

function recursiveIssuer(m) {
if (m.issuer) {
return recursiveIssuer(m.issuer);
Expand Down Expand Up @@ -123,7 +130,7 @@ export default class BaseOptimizer {
}

warmupThreadLoaderPool() {
const baseModules = ['babel-loader', BABEL_PRESET_PATH];
const baseModules = ['babel-loader', BABEL_PRESET_PATH, LEGACY_PRESETS];

threadLoader.warmup(
// pool options, like passed to loader options
Expand Down Expand Up @@ -522,6 +529,8 @@ export default class BaseOptimizer {
}

getPresets() {
return IS_CODE_COVERAGE ? [ISTANBUL_PRESET_PATH, BABEL_PRESET_PATH] : [BABEL_PRESET_PATH];
return IS_CODE_COVERAGE
? [ISTANBUL_PRESET_PATH, BABEL_PRESET_PATH, LEGACY_PRESETS]
: [BABEL_PRESET_PATH, LEGACY_PRESETS];
}
}
4 changes: 2 additions & 2 deletions src/optimize/create_ui_exports_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function normalizePath(path) {
return path.replace(/[\\\/]+/g, '/');
}

export default function() {
module.exports = function() {
if (!module.id.includes('?')) {
throw new Error('create_ui_exports_module loaded without JSON args in module.id');
}
Expand All @@ -37,4 +37,4 @@ export default function() {
return {
code: `${comment}\n${requires}\n`,
};
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ import {
} from './context/query';
import { callAfterBindingsWorkaround } from './context/helpers/call_after_bindings_workaround';

const module = getAngularModule();

module.directive('contextApp', function ContextApp() {
getAngularModule().directive('contextApp', function ContextApp() {
return {
bindToController: true,
controller: callAfterBindingsWorkaround(ContextAppController),
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.browser.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"target": "es5",
"target": "esnext",
"module": "esnext",
},
"include": [
Expand Down
2 changes: 2 additions & 0 deletions x-pack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@
},
"dependencies": {
"@babel/core": "^7.9.0",
"@babel/plugin-syntax-dynamic-import": "^7.8.3",
"@babel/plugin-transform-modules-commonjs": "^7.9.0",
"@babel/register": "^7.9.0",
"@babel/runtime": "^7.9.2",
"@elastic/apm-rum-react": "^1.1.1",
Expand Down