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

Implement resynchronization of the client and server #7631

Merged
merged 4 commits into from
Feb 21, 2020
Merged

Conversation

joheriks
Copy link
Contributor

@joheriks joheriks commented Feb 19, 2020

When the server receives the resync request, run detach listeners and mark all state tree nodes as not attached. Handle the changes rebuilding the state tree from the root on the client.

Fixes #7590.


This change is Reviewable

@joheriks joheriks marked this pull request as ready for review February 19, 2020 21:41
caalador
caalador previously approved these changes Feb 20, 2020
Copy link
Contributor

@caalador caalador left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 7 files at r1, 8 of 8 files at r2.
Reviewable status: :shipit: complete! all discussions resolved, 1 of 1 LGTMs obtained

OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from External Reviews to Reviewer approved Feb 20, 2020
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Reviewer approved to Changes Requested Feb 20, 2020
Copy link
Contributor Author

@joheriks joheriks left a comment

Choose a reason for hiding this comment

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

@Legioth highlighted another potential issue with @ClientCallable promises left hanging on the client-side after the resynchronization. Added a commit to reject them during resynchronization and an IT.

Reviewable status: all discussions resolved, 0 of 1 LGTMs obtained, and 1 stale (waiting on @caalador)

Copy link
Contributor

@caalador caalador left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r3.
Reviewable status: :shipit: complete! all discussions resolved, 1 of 1 LGTMs obtained

OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Changes Requested to Reviewer approved Feb 21, 2020
@joheriks joheriks merged commit 6b5ad96 into 2.2 Feb 21, 2020
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Reviewer approved to Done - pending release Feb 21, 2020
@joheriks joheriks deleted the issues/fix-7590 branch February 21, 2020 08:08
joheriks pushed a commit that referenced this pull request Feb 24, 2020
* Mark server-side state tree unattached and send all changes, clear client-side state tree and rebuild DOM 

* Reject any pending promises from @ClientCallable invocations during client resynchronization
@caalador caalador added this to the 2.2.0.alpha14 milestone Feb 25, 2020
joheriks pushed a commit that referenced this pull request Feb 25, 2020
* Mark server-side state tree unattached and send all changes, clear client-side state tree and rebuild DOM 

* Reject any pending promises from @ClientCallable invocations during client resynchronization
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
OLD Vaadin Flow ongoing work (Vaadin ...
  
Done - pending release
Development

Successfully merging this pull request may close these issues.

None yet

3 participants