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

[Flight] Fix Client components losing state on refetches #20319

Closed
wants to merge 2 commits into from

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Nov 23, 2020

Fixes test in #20318.

I bet there's some better fix. But the problem is that this trick causes creation of lazy wrappers on the client, which would be different between refetches. Maybe they should be cached globally? For now, removing this seems to work.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Nov 23, 2020
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b64032c:

Sandbox Source
React Configuration

@sizebot
Copy link

sizebot commented Nov 23, 2020

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against b64032c

@sizebot
Copy link

sizebot commented Nov 23, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against b64032c

Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are important. I’m surprised we don’t have tests.

Lazy elements exists in core exactly for the reason to enable them to reconcile so I’m sure this is just some oversight somewhere.

@sebmarkbage
Copy link
Collaborator

I think I meant to change the lazy type semantics to reconcile but I ended up doing it only for the element type.

We should do it for the element type too in childfiber.

There’s no reason the existing usage of lazy shouldn’t reconcile when the same component is imported using two different lazy wrappers.

@sebmarkbage
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants