Mantine >= 5.10.5 menu rendered with react-dom hydrate causes memory leak if transition props are provided

[aniravi24]

What package has an issue

@mantine/core

Describe the bug

Mantine 5.10.4 and below has no issues, but trying to open a Menu.Dropdown in Mantine 5.10.5 causes the browser to freeze and the menu doesn't open. Browser memory usage spikes from a couple hundred MB to multiple GB for that tab in Chrome if I keep the commit below.

What version of @mantine/hooks page do you have in package.json?

5.10.5

If possible, please include a link to a codesandbox with the reproduced problem

https://codesandbox.io/p/sandbox/morning-star-de8v1w

Do you know how to fix the issue

No

Are you willing to participate in fixing this issue and create a pull request with the fix

None

Possible fix

Reverting this commit fixes this issue.
#3576

[rtivital]

please provide a codesandbox (template– https://codesandbox.io/s/mantine-template-9ze89?file=/src/App.tsx:0-6) with minimal reproducible example https://stackoverflow.com/help/minimal-reproducible-example. It is not possible to identify the issue without reproduction.

[aniravi24]

Sorry for the delay! I was unable to reproduce it in a few codesandboxes but finally managed to reproduce with the Mantine Remix template.

https://codesandbox.io/p/sandbox/morning-star-de8v1w

A few notes from trying to debug this that might be helpful to you:

1. I use the deprecated hydrate API from React 18, and not hydrateRoot or the newer React 18 APIs because of some issues with Chrome extensions injecting scripts that haven't been resolved yet in React. The mantine template uses the same hydrate API. However, if you use hydrateRoot, the issue goes away. See the comments I left in entry.client.tsx to use the hydrateRoot API instead, this won't freeze the browser.
2. It only freezes with using a custom Menu.Target child that supports receiving a ref. If you don't have to pass down the ref, the dropdown shows up on the top left corner of the screen (since the ref is not correctly passed down), but the browser no longer hangs and the menu still works, just not attached to the target like it's supposed to be.
3. Also be prepared for your sandbox to freeze if you click on the menu, it will likely require restarting the sandbox.
4. I think render and createRoot from react-dom for create-react-app templates did work, but I would double check (if this even matters for debugging).

[aniravi24]

If you remove all the transition props, then it appears to work again. So for example, in my codesandbox above, remove all the props to Menu (transition, position, etc.) and it will work again. Same is true in Mantine 6 (no transition props being passed).

[cayter]

The same is happening to us, had to revert to 5.10.4 to workaround it.

[cyantree]

I might have a hunch but not a solution.

Hunch

The following line might be part of the problem and therefore part of the solution:
https://github.com/mantinedev/mantine/blob/master/src/mantine-core/src/Popover/PopoverDropdown/PopoverDropdown.tsx#L75

Background

Here's a bit background after digging through it:

• The issue seems to be related to the positioning algorithm from @floating-ui/react. It calculates the best placement for the given object. In your example this is bottom-start because the menu can open only to the bottom.
• However something triggers an endless loop because the calculated position alternates between bottom-start and top-start (top-start is configured via prop).
• Because the computed placement is used as a key on the element it gets recreated and therefore the reference changes.
• The PR [@mantine/core] Popover: fix floating-auto-update issue #3576 fixed the reference handling. So because of this the changed element will be recognized and triggers the auto update process which then somehow results in the endless loop.

Fixing the example

By changing the placement in your example to bottom-start the issue vanishes because the configured placement matches the computed one.

You can also add some space above the button (e. g. with mt={300}) and then use top-start without any problems.

A possible workaround

This also may be the workaround until there's a solution:

• Make sure your placement configuration matches the space that's available for the dropdown.

Solution

Maybe finding a better key would be a good fit because then the element wouldn't get recreated unnecessarily.

The key was added in #3065 so maybe @jvdsande can jump in here?

[jvdsande]

If I remember correctly where this is coming from, the key was needed for the correct computed position to be applied.

Since there has been some more work done on the positioning since then, the key might actually not be necessary at all, we could try removing it and see.

I'll dive back into my previous MR to remember exactly which case the key was fixing in the first place.

[cyantree]

Thanks for helping out so quickly. It's appreciated!
Looking forward to your findings. 👍

[jvdsande]

Preliminary investigation: the key is still needed for what it's been introduced for in the first place. Removing it, dynamically updating the position via props will fail on scroll/resize, the initial position is restored.

I'm going to dive deeper into this, coming up with a better key. The fact of the matter is, this needs to react to props changes, not internal computed position changes. I'll investigate further, using the reproduction as playground.

Naive solution: propagate the props-set position and use this as key.

Better solution: deep dive to understand why the initial bug is happening, and how to properly solve it without key.

[jvdsande]

Hi @aniravi24, after investigating I found a workaround removing the need of key, and that fixes the issue in your sandbox. I'll prepare a fix, however, this will most likely be implemented only in Mantine 6. Do you have any blockers for updating to v6?

I can provide a patch applicable through patch-package on latest v5 too.

[aniravi24]

Thank you all for diving into this.

@jvdsande I think I made all the code changes already to update to v6, so unless I find anything else that doesn't work (will put in a separate bug report if I do), implementing it in v6 works for me at least. Thank you!

[cyantree]

Great, thanks for taking care of this @jvdsande 👍

[vishnup-ditto]

Hi @aniravi24, after investigating I found a workaround removing the need of key, and that fixes the issue in your sandbox. I'll prepare a fix, however, this will most likely be implemented only in Mantine 6. Do you have any blockers for updating to v6?

I can provide a patch applicable through patch-package on latest v5 too.

Hey @jvdsande can you please share the patch-package changes. Can be helpful for people still in v5.