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

react-jss dynamic rules memory leak in strict mode #1648

Open
ernestostifano opened this issue Jun 22, 2023 · 0 comments
Open

react-jss dynamic rules memory leak in strict mode #1648

ernestostifano opened this issue Jun 22, 2023 · 0 comments

Comments

@ernestostifano
Copy link

ernestostifano commented Jun 22, 2023

Expected behavior:
Unreferenced style rules should be properly removed after un-mounting all associated components instances.

Describe the bug:
When React's strict mode is enabled, dynamic rules are not properly removed after un-mounting components, probably because of strict mode's double render (rule keys gets shifted during second render, probably because React Hooks best practices were not followed).

The app ultimately becomes unresponsive.

Reproduction:

https://codesandbox.io/s/happy-joliot-pmmq4d

It is sufficient to mount and un-mount a component with dynamic styles in any React application using strict mode and count the number of attached styles rules.

  1. Clone this simple reproduction repo.
  2. Checkout this branch: issue/react-jss-no-dynamic-remove
  3. Run yarn start
  4. Alternately click on Component A and Component B buttons and see how style rules count increases.
  5. Remove <StrictMode> from ./src/app.js and repeat step 4. No more leak.

Note: If a custom ID generator is added to print generated classnames, it can be seen how keys increase twice because of strict mode's double render.

Versions (please complete the following information):

  • jss: 10.10.0
  • Browser: Chrome, Firefox, Safari and Edge (latest versions at the time of submission)
  • OS: macOS 13.4

Managing expectations:
I think this is an important bug because it can be easily reproduced and ultimately makes apps unresponsive during development. Also, it was not that simple to pinpoint the source of the memory leak. Should not be that difficult to fix. I would be happy to work on the fix if someone gives me proper context.

Potentially related issues:

ernestostifano added a commit to ernestostifano/jss that referenced this issue Jul 12, 2023
ernestostifano added a commit to ernestostifano/jss that referenced this issue Jul 12, 2023
ernestostifano added a commit to ernestostifano/jss that referenced this issue Jul 13, 2023
ernestostifano added a commit to ernestostifano/jss that referenced this issue Jul 13, 2023
ernestostifano added a commit to ernestostifano/jss that referenced this issue Jul 13, 2023
ernestostifano added a commit to ernestostifano/jss that referenced this issue Sep 26, 2023
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

No branches or pull requests

1 participant