-
Notifications
You must be signed in to change notification settings - Fork 839
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 race condition in EuiIcon's internal setState #3118
Fix race condition in EuiIcon's internal setState #3118
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_3118/ |
1 similar comment
Preview documentation changes for this PR: https://eui.elastic.co/pr_3118/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for the quick attention to this. Really appreciate it.
jenkins test this flaky test |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3118/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks, @chandlerprall
@paul-tavares Is this something you need for 7.7, or will 7.8 be ok? |
@snide yeah, we have been backporting to 7.x (currently 7.7), so if you can get it there, that would be great. Once done, I will go back to our current implementation and add back in the |
Summary
Fixes #3113
Reproduced the issue in the Kibana branch as described. Copied code to EUI's docs to boil it down to a basic use case, and found:
empty
, which triggers a dynamic fetch for the empty iconimg
element created, network request madeAfter making this change in EUI & confirming it fixed my simplified test case, I built EUI with the change and dropped it into the above Kibana branch, confirming it resolves the issue there too.
Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Checked in IE11 and Firefox- [ ] Props have proper autodocs- [ ] Added documentation examples- [ ] Added or updated jest tests- [ ] Checked for accessibility including keyboard-only and screenreader modes