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

EuiIconClass re-rendering issue #6159

Closed
cdolek opened this issue Aug 23, 2022 · 6 comments · Fixed by #6165
Closed

EuiIconClass re-rendering issue #6159

cdolek opened this issue Aug 23, 2022 · 6 comments · Fixed by #6165

Comments

@cdolek
Copy link
Contributor

cdolek commented Aug 23, 2022

I'm working on an app that has an area where users can drag nodes around. At some point, we had performance issues and I had to debug the application by isolating each component. It turns out that some Eui components were being re-rendered unnecessarily.

I used why-did-you-render to look into what components were re-rendering and found out that EuiIconClass causes re-renders for no apparent reason. Really weird to see this, as no data is flowing to that component.

WDYR states the reason as "props changes" and "different objects that are equal by value." Props received are theme.colorMode: "DARK", theme.euiTheme: {"themeName": "EUI_THEME_AMSTERDAM", ..., theme.modifications: {}

I obviously want to prevent re-renders, not sure if it's about something the way EuiIconClass was structured or the way I'm using Eui in general. Appreciate any help resolving this problem.

This is basically how I use Eui:

const euiCache = createCache({
  key: 'eui',
  container: document.querySelector('meta[name="eui-style-insert"]') as Node,
});
euiCache.compat = true;
    <StyleSheetManager disableVendorPrefixes>
      <EuiProvider colorMode="dark" cache={euiCache}>
        <StyledThemeProvider theme={reactUITheme}>
          <GlobalStyle />
            <QueryClientProvider client={queryClient}>
              <RbacProvider registry={rbacRegistry}>
                <AppLoaderAndOutletWithUserProvider />
                  ...
                  ...

Main Component:

<>
  <Header /> // A header like https://elastic.github.io/eui/#/layout/header
  <Canvas /> // Where the dragging happens
</>

A simple example from Header, this would cause a re-render each time.

  • See, the EuiIconClass reported by WDYR is contained within other components and it's almost impossible to replace it to get rid of the problem even if I wanted to do that.
const Logo = () => (
  <EuiHeaderLogo iconType={logo} href="#" aria-label="Go to home page" onClick={preventDefault}>
    My Logo
  </EuiHeaderLogo>
);

Any thoughts or ideas? Thanks in advance!

@cee-chen
Copy link
Member

Pinging @chandlerprall on this one as he's currently looking at EuiIcon for an unrelated issue, and render/perf is also up his alley

@chandlerprall
Copy link
Contributor

I've confirmed a simple case (Logo component above) with no changing props does re-render EuiIcon when the owning component renders. Looking into why.

@cdolek
Copy link
Contributor Author

cdolek commented Aug 24, 2022

@chandlerprall I just tested v65.0.2 and can confirm that the issue above persists.

A few suspects from my use case:
EuiButtonIcon, EuiSwitch, EuiBadge, EuiHeaderLogo (with SVG logo provided)

@cee-chen
Copy link
Member

cee-chen commented Aug 24, 2022

v65.0.2 does not yet have the fix (#6165) - that patch release fixed another unrelated / more breaking issue.

@cdolek
Copy link
Contributor Author

cdolek commented Aug 26, 2022

v65.0.2 does not yet have the fix (#6165) - that patch release fixed another unrelated / more breaking issue.

@constancecchen any ideas when this fix would be released?

@cee-chen
Copy link
Member

Almost certainly next Tuesday! (Our normal weekly release cadence)

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

Successfully merging a pull request may close this issue.

3 participants