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.

Sign up for GitHub

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

[build] Removes commonjs transforms #66506

Merged
merged 6 commits into from
May 21, 2020
Merged

[build] Removes commonjs transforms #66506

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5b14d95
[build] Removes commonjs transforms
May 14, 2020
dca4cc1
Merge branch 'master' of github.com:elastic/kibana into pr/66506
spalger May 20, 2020
b4b9869
avoid separating definition and assignment
spalger May 20, 2020
776facd
run with dist:false so the test properly verified the cache is used
spalger May 20, 2020
54273b7
Updates APM import to not import across boundaries
May 21, 2020
ce43e3a
Merge branch 'master' into remove-commonjs-transform
elasticmachine May 21, 2020
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
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
},
"dependencies": {
"@babel/core": "^7.9.0",
"@babel/plugin-transform-modules-commonjs": "^7.9.0",
"@babel/register": "^7.9.0",
"@elastic/apm-rum": "^5.1.1",
"@elastic/charts": "19.2.0",
Expand Down
2 changes: 0 additions & 2 deletions packages/kbn-babel-preset/package.json
View file
Open in desktop
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
View file
Open in desktop
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
7 changes: 6 additions & 1 deletion packages/kbn-i18n/src/angular/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
* under the License.
*/

export { I18nProvider, I18nServiceType } from './provider';
export { I18nProvider } from './provider';

export { i18nFilter } from './filter';
export { i18nDirective } from './directive';

// re-export types: https://github.com/babel/babel-loader/issues/603
import { I18nServiceType as _I18nServiceType } from './provider';
export type I18nServiceType = _I18nServiceType;
4 changes: 3 additions & 1 deletion packages/kbn-i18n/src/react/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
* under the License.
*/

import { InjectedIntl as _InjectedIntl } from 'react-intl';
export type InjectedIntl = _InjectedIntl;

export {
intlShape,
InjectedIntl,
FormattedDate,
FormattedTime,
FormattedRelative,
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-interpreter/.babelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"presets": ["@kbn/babel-preset/webpack_preset"],
"plugins": [
"@babel/plugin-transform-modules-commonjs",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This project is required from both server/client code and only has a single build output - a follow-up could be to build separate packages but out of the scope for this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, yeah, it used to build separate outputs...

Copy link
Member

Choose a reason for hiding this comment

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

The plan is to remove this package eventually as the expressions plugin should already be exposing all of this functionality.

["@babel/plugin-transform-runtime", {
"regenerator": true
}]
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-interpreter/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"devDependencies": {
"@babel/cli": "^7.8.4",
"@babel/core": "^7.9.0",
"@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
10 changes: 5 additions & 5 deletions packages/kbn-optimizer/src/integration_tests/__snapshots__/basic_optimization.test.ts.snap
View file
Open in desktop

Large diffs are not rendered by default.

56 changes: 35 additions & 21 deletions packages/kbn-optimizer/src/integration_tests/basic_optimization.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ const MOCK_REPO_DIR = Path.resolve(TMP_DIR, 'mock_repo');

expect.addSnapshotSerializer(createAbsolutePathSerializer(REPO_ROOT));

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);
},
},
});

beforeAll(async () => {
await del(TMP_DIR);
await cpy('**/*', MOCK_REPO_DIR, {
Expand All @@ -52,23 +65,11 @@ 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');

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);
},
},
});
const msgs = await runOptimizer(config)
.pipe(logOptimizerState(log, config), toArray())
.toPromise();
Expand Down Expand Up @@ -125,13 +126,6 @@ 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');

const foo = config.bundles.find(b => b.id === 'foo')!;
expect(foo).toBeTruthy();
foo.cache.refresh();
Expand Down Expand Up @@ -174,7 +168,7 @@ it('uses cache on second run and exist cleanly', async () => {
repoRoot: MOCK_REPO_DIR,
pluginScanDirs: [Path.resolve(MOCK_REPO_DIR, 'plugins')],
maxWorkerCount: 1,
dist: true,
dist: false,
});

const msgs = await runOptimizer(config)
Expand All @@ -200,6 +194,26 @@ it('uses cache on second run and exist cleanly', async () => {
`);
});

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,
});

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');
});

/**
* Verifies that the file matches the expected output and has matching compressed variants.
*/
Expand Down
5 changes: 3 additions & 2 deletions src/legacy/core_plugins/timelion/public/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import 'angular-sanitize';

import { i18n } from '@kbn/i18n';

import routes from 'ui/routes';
import { capabilities } from 'ui/capabilities';
import { docTitle } from 'ui/doc_title';
import { fatalError, toastNotifications } from 'ui/notify';
Expand Down Expand Up @@ -62,9 +63,9 @@ document.title = 'Timelion - Kibana';

const app = require('ui/modules').get('apps/timelion', ['i18n', 'ngSanitize']);

require('ui/routes').enable();
routes.enable();

require('ui/routes').when('/:id?', {
routes.when('/:id?', {
template: rootTemplate,
reloadOnSearch: false,
k7Breadcrumbs: ($injector, $route) =>
Expand Down
10 changes: 8 additions & 2 deletions src/optimize/base_optimizer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ 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')],
};

function recursiveIssuer(m) {
if (m.issuer) {
return recursiveIssuer(m.issuer);
Expand Down Expand Up @@ -123,7 +127,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 +526,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
View file
Open in desktop
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`,
};
}
};
4 changes: 1 addition & 3 deletions src/plugins/discover/public/application/angular/context_app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,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 yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@
dependencies:
"@babel/helper-plugin-utils" "^7.0.0"

"@babel/plugin-syntax-dynamic-import@^7.2.0", "@babel/plugin-syntax-dynamic-import@^7.8.0", "@babel/plugin-syntax-dynamic-import@^7.8.3":
"@babel/plugin-syntax-dynamic-import@^7.2.0", "@babel/plugin-syntax-dynamic-import@^7.8.0":
version "7.8.3"
resolved "https://registry.yarnpkg.com/@babel/plugin-syntax-dynamic-import/-/plugin-syntax-dynamic-import-7.8.3.tgz#62bf98b2da3cd21d626154fc96ee5b3cb68eacb3"
integrity sha512-5gdGbFon+PszYzqs83S3E5mpi7/y/8M9eC90MRTZfduQOYW76ig6SOSPNe41IG5LoP3FGBn2N0RjVDSQiS94kQ==
Expand Down