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

[dll] run babel on all modules that aren't explicitly excluded #59923

Closed
wants to merge 2 commits into from

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Mar 11, 2020

Fixes #59833

While trying to find a solution that allows us to validate that our DLL bundles don't include JS that breaks IE I started digging into possibly building a custom Webpack plugin and/or loader. During this investigation I realized something... Webpack is already parsing all of the code in our node_modules in order to determine dependencies and my fears of the performance impact of running node_modules through babel might be misplaced. I then decided to test out the performance impact of transpiling all modules in the DLL with babel and I'm pretty sure we don't actually see that much of a performance impact (like 2-4 seconds in dev).

I'm pushing this change up with shame, as I've been pushing back on making this change for so long and pushed a lot of people to work around this, but hopefully people will forgive me...

Also, I'd like to make sure that performance is still manageable for other team members as I'm testing on an iMac Pro.

@spalger spalger added discuss Team:Operations Team label for Operations Team labels Mar 11, 2020
@elasticmachine
Copy link
Contributor

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

@spalger spalger mentioned this pull request Mar 11, 2020
7 tasks
@kibanamachine
Copy link
Contributor

💔 Build Failed


Test Failures

Kibana Pipeline / x-pack-intake-agent / X-Pack Jest Tests.x-pack/plugins/index_management/__jest__/client_integration. on component mount review (step 5) should render a warning message if a wildcard is used as an index pattern

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 3 times on tracked branches: https://github.com/elastic/kibana/issues/59859


Stack Trace

TypeError: Cannot read property 'data' of undefined
    at getData (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/index_management/public/application/components/mappings_editor/mappings_state.tsx:120:50)
    at setDataGetter (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/index_management/public/application/components/template_form/steps/step_mappings.tsx:33:22)

Kibana Pipeline / x-pack-intake-agent / X-Pack Jest Tests.x-pack/plugins/index_management/__jest__/client_integration. on component mount form payload & api errors should surface the API errors from the put HTTP request

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 1 times on tracked branches: https://github.com/elastic/kibana/issues/59869


Stack Trace

Error: Method “simulate” is meant to be run on 1 node. 0 found instead.
    at ReactWrapper.single (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/enzyme/src/ReactWrapper.js:1168:13)
    at ReactWrapper.simulate (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/enzyme/src/ReactWrapper.js:665:17)
    at Object.completeStepFour (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/index_management/__jest__/client_integration/helpers/template_form.helpers.ts:133:30)
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/index_management/__jest__/client_integration/template_create.test.tsx:235:25

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

@spalger
Copy link
Contributor Author

spalger commented Mar 11, 2020

Turns out, I missed an incredibly important bit of code that lead me to the think that running node_modules through babel would be okay:

const BABEL_EXCLUDE_RE = [/[\/\\](webpackShims|node_modules|bower_components)[\/\\]/];

This regular exception was still keeping everything out of the babel loader and as soon as node_modules is removed from that regex the optimizer slows to a crawl and fails to succeed before OOM-ing the process.

@spalger spalger closed this Mar 11, 2020
@spalger spalger deleted the implement/babel-the-dll branch March 11, 2020 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Team:Operations Team label for Operations Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Look for invalid syntax in bundles in build process
3 participants