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

Unnecessary unmounting/mounting children with keys #2783

Closed
yishn opened this issue Oct 3, 2020 · 8 comments · Fixed by #4010
Closed

Unnecessary unmounting/mounting children with keys #2783

yishn opened this issue Oct 3, 2020 · 8 comments · Fixed by #4010
Labels

Comments

@yishn
Copy link

yishn commented Oct 3, 2020

Hi all, I'm the author of tikzcd-editor and am experiencing severe performance degradation after updating Preact recently.

tikzcd-editor is rendering a grid of elements onto the viewport; we only render visible grid cells. The user can use the pan tool to pan the grid, so cells outside of the viewport need to be added while panning, and cells that go outside of the viewport get removed.

Each grid cell will be typeset using MathJax on every change, which is expensive, that's why every grid cell has a key set to its coordinates, so we avoid rerendering while panning. This worked fine in v10.4.1, but in v10.4.2 and the latest version v10.5.3, we experience a lot of unnecessary rerenders.

Reproduction

Steps to reproduce

Here's a deployment of tikzcd-editor with v10.5.3 with a sample diagram. When trying to pan the viewport, you can see it's not smooth at all. When opening the Devtools you can see that Preact is unnecessarily unmounting/mounting elements.

Expected Behavior

Visible children should not be unmounted/remounted like in v10.4.1:

Good behavior

Actual Behavior

A lot of unnecessary unmounting/rendering of children as experienced in v10.4.2 and v10.5.3:

Bad behavior

@yishn yishn changed the title Unnecessary unmounting/mounting children with keys in v10.4.2 Unnecessary unmounting/mounting children with keys Oct 3, 2020
@marvinhagemeister
Copy link
Member

Probably related to #2619

@darsain
Copy link

darsain commented Nov 6, 2020

Was told my issues is related, so here is my case report:

Whenever you remove an uncertain number of elements from a keyed list, the elements after them get remounted. The number seems to differ between environments. On my local app it's 20+, on the isolated example below it's 50+.

Here is a visualization of the issue. Elements that get remounted flash red. The last 20 items should never flash, as only range 20-80 is being toggled:

xG61GfMSA5

This doesn't happen for smaller amount of items being removed:

VVaLqUrt3k

The issue is isolated here: https://codesandbox.io/s/cocky-joliot-9gw5v

@pavulon
Copy link

pavulon commented Nov 16, 2020

In your case @darsain, the magic number is more than a half. Probably this has something to do with it.

@developit
Copy link
Member

Echoing from various discussions elsewhere: the j/2 child search optimization was never supposed to apply to keyed matches, which to me makes this a bug rather than just a performance degradation.

@cpatti97100
Copy link

good evening all :) is there undergoing work about this? thanks!

@jaydenseric
Copy link

This issue causes serious FOUC problems when rendering arrays of link DOM nodes for stylesheets and something early in the list changes, see:

#3285 (comment)

What is an ETA for a fix? Is it fixed in main branch already? Will it ship in Preact v11, or in a v10 patch? If there is a prerelease version with the fix, is there a way to use it via JSPM CDN? Are there any known workarounds?

@JoviDeCroock
Copy link
Member

@jaydenseric this has been fixed in v11, introducing this fix in v10 would be a huge perf impediment I've tried in #3359 for v11 with our new diffing algo it is a net positive #3358

@jaydenseric
Copy link

That's great news, when do you think v11 will be released? I'm currently working full time on a buildless SSR web app framework for Deno based around Preact, and this issue is a blocker. If there is no way to try a v11 prerelease, and a stable v11 if far away, I might have to reschedule my next few weeks.

Also, I'm keen to contribute to improving Preact, particularly regarding Deno compatibility. Is the main branch the one to PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment