Skip to content

[build] Removes commonjs transforms#66506

Merged
tylersmalley merged 6 commits intoelastic:masterfrom
tylersmalley:remove-commonjs-transform
May 21, 2020
Merged

[build] Removes commonjs transforms#66506
tylersmalley merged 6 commits intoelastic:masterfrom
tylersmalley:remove-commonjs-transform

Conversation

@tylersmalley
Copy link
Copy Markdown
Member

@tylersmalley tylersmalley commented May 14, 2020 •
edited
Loading

Blocker for tree shaking

It's not necessary for us to transform to commonjs, and removing the step reduces the builds by ~27%

# averages over three runs 227.27 (master) to 164.7
for i in {1..3}; do time node scripts/build_kibana_platform_plugins.js --no-cache --workers=6; done

@tylersmalley tylersmalley force-pushed the remove-commonjs-transform branch 14 times, most recently from b724a7a to 6c58e26 Compare May 20, 2020 12:56
@tylersmalley
[build] Removes commonjs transforms
5b14d95 
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley tylersmalley force-pushed the remove-commonjs-transform branch from 6c58e26 to 5b14d95 Compare May 20, 2020 19:41
tylersmalley
tylersmalley commented May 20, 2020
View reviewed changes
packages/kbn-interpreter/.babelrc
{
"presets": ["@kbn/babel-preset/webpack_preset"],
"plugins": [
"@babel/plugin-transform-modules-commonjs",
Copy link
Copy Markdown
Member Author

@tylersmalley tylersmalley May 20, 2020

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
Copy Markdown
Contributor

@spalger spalger May 20, 2020

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
Copy Markdown
Contributor

@lukeelmers lukeelmers May 20, 2020

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.

@tylersmalley tylersmalley added v7.8.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Operations Kibana-Operations Team labels May 20, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented May 20, 2020

Pinging @elastic/kibana-operations (Team:Operations)

@tylersmalley tylersmalley marked this pull request as ready for review May 20, 2020 19:56
@tylersmalley tylersmalley requested a review from a team May 20, 2020 19:56
@tylersmalley tylersmalley requested review from a team as code owners May 20, 2020 19:56
@tylersmalley tylersmalley mentioned this pull request May 20, 2020
Closed
2 tasks
@tylersmalley
Updates APM import to not import across boundaries
54273b7 
Introduced in elastic#66432 and I have a more thorough PR to prevent imports in
server from public in elastic#67149

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley tylersmalley requested a review from a team May 21, 2020 05:30
@botelastic botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label May 21, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented May 21, 2020

Pinging @elastic/apm-ui (Team:apm)

@tylersmalley
Copy link
Copy Markdown
Member Author

tylersmalley commented May 21, 2020

@elasticmachine merge upstream

majagrubic
majagrubic approved these changes May 21, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@majagrubic majagrubic left a comment

Choose a reason for hiding this comment

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

kibana-app changes look good!

sorenlouv
sorenlouv approved these changes May 21, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

APM changes lgtm

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented May 21, 2020

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@tylersmalley tylersmalley merged commit c675301 into elastic:master May 21, 2020
@tylersmalley tylersmalley mentioned this pull request May 21, 2020
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented May 25, 2020

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 66506 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 25, 2020
tylersmalley added a commit to tylersmalley/kibana that referenced this pull request May 26, 2020
@tylersmalley @spalger @elasticmachine
[build] Removes commonjs transforms (elastic#66506)
880779a 
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	packages/kbn-optimizer/src/integration_tests/basic_optimization.test.ts
#	src/optimize/base_optimizer.js
#	src/optimize/create_ui_exports_module.js
@tylersmalley tylersmalley mentioned this pull request May 26, 2020
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented May 27, 2020

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

6 similar comments
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented May 28, 2020

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented May 29, 2020

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jun 1, 2020

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jun 2, 2020

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jun 3, 2020

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jun 4, 2020

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

spalger added a commit that referenced this pull request Jun 4, 2020
@tylersmalley @spalger @elasticmachine
[7.x] [build] Removes commonjs transforms (#66506) (#67375)
22c539f 
Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 4, 2020
tylersmalley added a commit that referenced this pull request Jun 10, 2020
@tylersmalley
[eslint] Prevents importing of public code into server (#67149)
8a5a7c3 
We should not be allowing importing of public into server. Any shared code should reside in a common directory. After #66506, this will not even be possible as we will no longer be transpiling public code into commonjs.

Blocks #66506

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
tylersmalley added a commit to tylersmalley/kibana that referenced this pull request Jun 10, 2020
@tylersmalley
[eslint] Prevents importing of public code into server (elastic#67149)
e1191d7 
We should not be allowing importing of public into server. Any shared code should reside in a common directory. After elastic#66506, this will not even be possible as we will no longer be transpiling public code into commonjs.

Blocks elastic#66506

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
tylersmalley added a commit that referenced this pull request Jun 10, 2020
@tylersmalley @elasticmachine
[eslint] Prevents importing of public code into server (#67149) (#68782)
a8718ef 
We should not be allowing importing of public into server. Any shared code should reside in a common directory. After #66506, this will not even be possible as we will no longer be transpiling public code into commonjs.

Blocks #66506

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@tylersmalley tylersmalley mentioned this pull request Jul 29, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lukeelmers lukeelmers lukeelmers approved these changes

+3 more reviewers

@sorenlouv sorenlouv sorenlouv approved these changes

@spalger spalger spalger approved these changes

@majagrubic majagrubic majagrubic approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Feature:Build Tooling release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:Operations Kibana-Operations Team v7.9.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@tylersmalley @elasticmachine @kibanamachine @sorenlouv @spalger @lukeelmers @majagrubic