eui icon indicating copy to clipboard operation
eui copied to clipboard

[Platform] Update browser targets to reduce build size

Open chandlerprall opened this issue 3 years ago • 1 comments

Summary

Fixes an issue that has come up twice in Kibana support. The issue is that the spread operation,

matchingOptions.push(...matchingOptionsForGroup);

was transpiled to

matchingOptions.push.apply(matchingOptions, matchingOptionsForGroup);

which overflowed the call stack size when matchingOptionsForGroup array was too large (see https://mathiasbynens.be/demo/javascript-argument-count). Initial testing showed the spread operation itself didn't have the same limit, so I updated the babel plugins to the latest version and optimized our @babel/env browser list (dropped phantomjs which was used for the defunct visual regression testing) so the spread operation wouldn't be transpiled. However I ran into the same error with an even larger array size, so I ended up refactoring to use the concat approach.

With the babel & browser target changes, es build artifact drops 8.0M -> 7.4M. I'll test this further in Kibana & go through the EUI docs in all browsers before merging, but wanted to put it in front of folks' eyes before that continued testing.

Changes

  • updated array building method in matching_options.ts
  • added a regression unit test
  • updated babel modules to latest
  • minor yarn.lock clean up
  • removed phantomjs support
  • removed @babel/env target override in jest's babel config as it reads from .browserslistrc instead

/cc @flash1293

Checklist

~- [ ] Checked in both light and dark modes~

  • [ ] Checked in mobile
  • [ ] Checked in Chrome, Safari, Edge, and Firefox ~- [ ] Props have proper autodocs and playground toggles~ ~- [ ] Added documentation~ ~- [ ] Checked Code Sandbox works for any docs examples~ ~- [ ] Added or updated jest and cypress tests~
  • [ ] Checked for breaking changes and labeled appropriately ~- [ ] Checked for accessibility including keyboard-only and screenreader modes~ ~- [ ] Updated the Figma library counterpart~
  • [ ] A changelog entry exists and is marked appropriately

chandlerprall avatar Jun 15 '22 17:06 chandlerprall

Per Constance's request, I'm splitting the combobox changes out to a separate PR and will make this one the transpilation target update for wider testing

chandlerprall avatar Jun 16 '22 15:06 chandlerprall

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

github-actions[bot] avatar Sep 15 '22 00:09 github-actions[bot]

Gonna close this one, it's out of date and should start fresh

chandlerprall avatar Sep 15 '22 20:09 chandlerprall