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

Fix EuiIcon so webpack can build dynamic require contexts #1998

Merged

Conversation

pugnascotia
Copy link
Contributor

@pugnascotia pugnascotia commented Jun 5, 2019

Summary

The work to remove TSLint in favour of ESLint has the side effect of "fixing" a lint failure in EuiIcon. It changed a plain string concatenation into a string template. Normally this would be fine, but this particular string concatenation is used to load icons dynamically, and Webpack can't build a dynamic require context from the compiled version of the template string.

Revert the change and add a lint directive to stop it recurring.

I built and linked the package, and tried it out in Cloud UI to ensure it was working as expected.

Checklist

  • This was checked in mobile
  • This was checked in IE11
  • This was checked in dark mode
  • Any props added have proper autodocs
  • Documentation examples were added
  • A changelog entry exists and is marked appropriately
  • This was checked for breaking changes and labeled appropriately
  • Jest tests were updated or added to match the most common scenarios
  • This was checked against keyboard-only and screenreader scenarios
  • This required updates to Framer X components

@pugnascotia pugnascotia added the bug label Jun 5, 2019
@pugnascotia pugnascotia changed the title Fix EuiIcon again so that webpack can build dynamic require contexts Fix EuiIcon again so that webpack can build dynamic require contexts Jun 5, 2019
Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

LGTM

I'm likely going to release 11.3.1 today and this will be in it

@pugnascotia pugnascotia changed the title Fix EuiIcon again so that webpack can build dynamic require contexts Fix EuiIcon so webpack can build dynamic require contexts Jun 5, 2019
@pugnascotia pugnascotia merged commit a470aee into elastic:master Jun 5, 2019
@pugnascotia pugnascotia deleted the fix-euiicon-webpack-loading branch June 5, 2019 13:33
@pugnascotia
Copy link
Contributor Author

Nice one Greg, please let @claracruz know when you've released it.

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

Successfully merging this pull request may close these issues.

2 participants