Skip to content

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

Closed
spalger wants to merge 2 commits intoelastic:masterfrom
spalger:implement/babel-the-dll
Closed

[dll] run babel on all modules that aren't explicitly excluded#59923
spalger wants to merge 2 commits intoelastic:masterfrom
spalger:implement/babel-the-dll

Conversation

@spalger
Copy link
Copy Markdown
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 Kibana-Operations Team labels Mar 11, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Mar 11, 2020

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

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

kibanamachine commented Mar 11, 2020

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

spalger commented Mar 11, 2020 •
edited
Loading

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:

kibana/src/optimize/dynamic_dll_plugin/dll_config_model.js

Line 45 in db228dd

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

Reviewers

No reviews

Assignees

No one assigned

Labels

discuss Team:Operations Kibana-Operations Team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Look for invalid syntax in bundles in build process

3 participants

@spalger @elasticmachine @kibanamachine