[Emotion] Add HOC util for memoized styles + memoize EuiIcon styles

[cee-chen]

Summary

This PR is a performance enhancement (+ minor code cleanup) on the EuiIcon component, and should not regress or change any output UI or generated classNames.

As always, I recommend code reviewing by commit.

QA

• https://eui.elastic.co/pr_7544/#/display/icons looks the same as production

General checklist

• Browser QA
  • Checked in both light and dark modes
  • Checked in Chrome, Safari, Edge, and Firefox
  • Checked for accessibility including keyboard-only and screenreader modes
    - [ ] Checked in mobile
• Docs site QA - N/A
• Code quality checklist
  • [ ] Added or updated jest and cypress tests Should be fine if tests pass
• Release checklist - N/A
  • [ ] A changelog entry exists and is marked appropriately. - Skipping the changelog on these PRs as they shouldn't affect either end-users or consumers
• Designer checklist - N/A

[kibanamachine]

Preview staging links for this PR:

• Docs site: https://eui.elastic.co/pr_7544/
• Storybook: https://eui.elastic.co/pr_7544/storybook

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 6b2adc0

[mgadewoll]

From what I can tell so far: LGTM! 🚀

[cee-chen]

Thanks Lene! Does the HOC wrapper itself make sense to you, as well as the new DRYed out getMemoizedStyles utility? To be honest, I wish there was a way to avoid HOCs (e.g. using a render prop instead) but I couldn't think of an easy way to do so.

Also, it would be nice to imagine us one day moving away from class components entirely, but I'm not sure we'll ever be able to do so given the complexity of some of our components.

[mgadewoll]

@cee-chen Yes, to me the updates make sense. While HOCs add extra weight here, I don't think we have another way to add the memoization logic without introducing some separate way just for class components. And as the idea is to (hopefully) remove class components altogether in the future and we want to keep things DRY, it definitely seems better to reuse the hooks via HOCs, imho 👍

[tkajtoch]

This results in the HoC component using the same display name as the wrapped component, which is confusing in React DevTools and may seem like improperly nested icons.

Could you please rename the HoC's displayName to match the recommended HoCName(WrappedComponentName) convention? In this case, that would be something like WithEuiStylesMemoizer(${Component.displayName || Component.name || 'Component'}). LMK what you think!

[cee-chen]

This is a copy of what we're already doing in withEuiTheme:

eui/src/services/theme/hooks.tsx

Lines 67 to 84 in 829709e

	export const withEuiTheme = <T extends {} = {}, U extends {} = {}>(
	Component: React.ComponentType<T & WithEuiThemeProps<U>>
	) => {
	const componentName =
	Component.displayName || Component.name || 'ComponentWithTheme';
	const Render = (
	props: Omit<T, keyof WithEuiThemeProps<U>>,
	ref: React.Ref<Omit<T, keyof WithEuiThemeProps<U>>>
	) => {
	const theme = useEuiTheme<U>();
	return <Component theme={theme} ref={ref} {...(props as T)} />;
	};
	const WithEuiTheme = forwardRef(Render);
	WithEuiTheme.displayName = componentName;
	return WithEuiTheme;

I'm reluctant to change it for that reason, and also primarily because I believe it will massively fuck up snapshots (which was likely the original reason for Greg doing this weird shenanigans in the first place). I don't love it, but since we're already doing it and I don't really feel like going through a massively painful set of Kibana upgrades, I'd rather not change it.

[tkajtoch]

I get it, but that's a new HoC, so enzyme snapshots will be affected anyway, won't they? DOM snapshots should all be fine since they aren't aware of React vDOM structure

[cee-chen]

I get it, but that's a new HoC, so enzyme snapshots will be affected anyway, won't they?

No, they won't, because:

1. The .displayName is overridden to use the original component displayName
2. We're replacing withEuiTheme with withEuiStylesMemoizer - that's the whole point of keeping the override the same

React DevTools is not the same as Enzyme's shallow/mounted renderer and I believe this code was originally written with Enzyme in mind (bleh).

[tkajtoch]

aah gotcha, I missed the withEuiTheme diff part! I'm good leaving it as is then and hope enzyme snapshots will comply 🤣