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

Upgrade EUI to 11.0.1; support dynamic import() calls in UI code + eui #36316

Merged
merged 18 commits into from
Jun 3, 2019

Conversation

chandlerprall
Copy link
Contributor

@chandlerprall chandlerprall commented May 8, 2019

Summary

  • Added support for dynamic import() calls to jest's babel configuration
  • Configured jest's transforms to apply to node_modules/@elastic/eui
  • Couple code changes from updated EUI type defs
  • Snapshot changes are all expected icon svg updates.
  • Updated one functional test to use a better selector when interacting with EUI's combo box (thanks @kobelb !)

Because of the widespread presence/usage of EuiIcon and the impact this most likely has on active PRs, after approvals I will merge master into this branch, re-update and check snapshots, and then merge.

EUI Changelog

11.0.1

Bug fixes

  • Fixed EuiIconTip's typescript definition (#1934)
  • Reinstated EuiIcon component ability to handle type prop updates (#1935)

11.0.0

  • Added support for custom React SVG elements and external SVG URLs to EuiIcon (#1924)

Bug fixes

  • Fixed Firefox flash of unstyled select dropdown (#1927)

Breaking changes

  • Split EuiIcon icon loading into dynamic imports (#1924)

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

💔 Build Failed

@chandlerprall
Copy link
Contributor Author

jenkins test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@chandlerprall chandlerprall requested a review from a team as a code owner May 10, 2019 17:24
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@chandlerprall
Copy link
Contributor Author

jenkins test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@chandlerprall
Copy link
Contributor Author

jenkins test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@chandlerprall
Copy link
Contributor Author

jenkins test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@chandlerprall
Copy link
Contributor Author

with all tests happy, this is now ready for review /me wipes sweat from brow

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM for babel/jest changes

plugins: [
// enables jest to parse and execute dynamic import() calls
'@babel/plugin-syntax-dynamic-import',
'dynamic-import-node'
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't hate with this, but I would really appreciate not needing to build EUI when running our tests. Ideally EUI can ship with a commonjs version in the future that applies this plugin to rewrite the dynamic imports with static ones, then we can use jest's moduleNameMapper config to rewrite @elastic/eui(/.*)? to <rootDir>/node_modules/@elastic/eui/commonjs$1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@chandlerprall chandlerprall merged commit 86fea48 into elastic:master Jun 3, 2019
chandlerprall added a commit to chandlerprall/kibana that referenced this pull request Jun 3, 2019
elastic#36316)

* Upgrade EUI to 11.0.1; support dynamic import() calls in UI code + eui

* update snaps

* Clicking on the svg itself once loaded

* updated snaps

* update icon in snapshots

* Fix snapshot
chandlerprall added a commit that referenced this pull request Jun 3, 2019
#36316) (#37900)

* Upgrade EUI to 11.0.1; support dynamic import() calls in UI code + eui

* update snaps

* Clicking on the svg itself once loaded

* updated snaps

* update icon in snapshots

* Fix snapshot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants