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

Do Not Unmount Old View On Reconnect #760

Closed
rmorshea opened this issue Jun 13, 2022 · 3 comments · Fixed by #886
Closed

Do Not Unmount Old View On Reconnect #760

rmorshea opened this issue Jun 13, 2022 · 3 comments · Fixed by #886
Labels
priority-1-high Should be resolved ASAP. type-revision About a change in functionality or behavior

Comments

@rmorshea
Copy link
Collaborator

rmorshea commented Jun 13, 2022

Current Situation

Presently, upon reconnecting, the client unmounts the old view and mounts a new one. This results in a brief, but jarring flash as the new view loads.

Proposed Actions

Instead of unmounting the old view we can simply retain it. The first message from the server should be the new VDOM which should be set at the root. This will cause React to render the full tree of components, but it will prevent the view from flashing from the user's perspective.

@rmorshea rmorshea added priority-2-moderate Should be resolved on a reasonable timeline. type-revision About a change in functionality or behavior labels Jun 13, 2022
@rmorshea rmorshea added this to the 1.0 milestone Jun 13, 2022
@Archmonger
Copy link
Contributor

Archmonger commented Jan 24, 2023

Could not unmounting the view cause a memory leak?

@rmorshea
Copy link
Collaborator Author

rmorshea commented Jan 24, 2023

I don't think so. As a note for this issue, it looks like ReactDOM.render is deprecated in favor of createRoot which returns an object with render and unmount methods. The docs state that calling render multiple times is allowed:

If you call render on the same root more than once, React will update the DOM as necessary to reflect the latest JSX you passed. React will decide which parts of the DOM can be reused and which need to be recreated by “matching it up” with the previously rendered tree. Calling render on the same root again is similar to calling the set function on the root component: React avoids unnecessary DOM updates.

@rmorshea rmorshea mentioned this issue Feb 1, 2023
3 tasks
@Archmonger Archmonger reopened this Feb 1, 2023
@Archmonger Archmonger modified the milestones: 1.0, Essential Feb 1, 2023
@rmorshea rmorshea removed this from the Essential milestone Feb 21, 2023
@Archmonger Archmonger added priority-1-high Should be resolved ASAP. and removed priority-2-moderate Should be resolved on a reasonable timeline. labels Jun 14, 2023
@Archmonger
Copy link
Contributor

Archmonger commented Sep 15, 2023

This issue was resolved in #886 and #901.

There is still the separate issue of the initial mount deleting the inner contents of the div, which can complicate #578.

Closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-1-high Should be resolved ASAP. type-revision About a change in functionality or behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants