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

Allow fallback of Suspense to be mounted multiple times #1859

Closed
wants to merge 2 commits into from

Conversation

sventschui
Copy link
Member

@sventschui sventschui commented Aug 12, 2019

I failed to reproduce #1813 in our demo app but with some debugging I came across the fact that the _dom property was missing on the fallback vnode. The reason for that seems to be the coerceToVNode in render since it clones the vnode. What I do not get is why I can not reproduce this in the demo app.

This PR adds 14B to preact and compat each.

Anybody has any ideas how to repro in demo?

TODO:

  • Tests
  • Revert demo

aduh95 added a commit to aduh95/preact that referenced this pull request Aug 13, 2019
It seems there is some kind of race condition here,
because sometimes the loading elements disapear, sometimes they don't

This also put in evidence a problem with nested Suspence components

Refs: preactjs#1859
Refs: preactjs#1813
aduh95 added a commit to aduh95/preact that referenced this pull request Aug 13, 2019
It seems there is some kind of race condition here,
because sometimes the loading elements disappear, sometimes they don't.

This also puts in evidence a problem with nested Suspence components.

Refs: preactjs#1859
Refs: preactjs#1813
aduh95 added a commit to aduh95/preact that referenced this pull request Aug 13, 2019
It seems there is some kind of race condition here,
because sometimes the loading elements disappear, sometimes they don't.

This also puts in evidence a problem with nested Suspence components.

Refs: preactjs#1859
Refs: preactjs#1813
@coveralls
Copy link

coveralls commented Aug 14, 2019

Coverage Status

Coverage increased (+0.1%) to 99.879% when pulling d884487 on bugfix/suspense-fallback-mounted-twice into 4b9e44b on master.

@sventschui
Copy link
Member Author

sventschui commented Aug 18, 2019

Just as a short update. I found that the duplicate dom of some components can be prevented by adding a guard to _childDidSuspend that prevents tracking promises multiple times. It does not solve the stale fallback dom to still be present.

During these investigations I found further issues with dom nodes that are mounted as siblings to a suspending component. As a result of these issues I started investigating two-phase rendering.

TL;DR I looked into the issue and think Suspense is not possible with the current implementation of preact. The two phase render will take a lot of time to be built and will land post 10.0.0 release

@sventschui
Copy link
Member Author

Closing this one as we need a two-phase diff algo to properly solve these issues

@sventschui sventschui closed this Sep 8, 2019
@ForsakenHarmony ForsakenHarmony deleted the bugfix/suspense-fallback-mounted-twice branch May 21, 2021 22:16
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 this pull request may close these issues.

None yet

2 participants