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

@PreserveOnRefresh doesn't preserve UI scoped beans #293

Closed
edler-san opened this issue Aug 7, 2019 · 5 comments
Closed

@PreserveOnRefresh doesn't preserve UI scoped beans #293

edler-san opened this issue Aug 7, 2019 · 5 comments

Comments

@edler-san
Copy link

The behaviour of @PreserveOnRefresh+CDI within Vaadin 14 differs from that of Vaadin 8. In the current version scope'd beans can differ after a refresh which did not happen in V8.

Related: vaadin/spring#473

@denis-anisimov
Copy link

See the user level explanation here:
vaadin/spring#473 (comment)

@denis-anisimov
Copy link

The solution is: UIScope should not work as it's expected here. See explanation above.

But there is future way to do it

There is already RouteScoped annotation which should be used for this.
But its implementation is UIScope based. So it won't work. And this is a bug.
We should make it preserving scoped beans on refresh if the owner has @PreserveOnRefresh annotation.

So the implementation should be changed.
See the implementation details here: vaadin/spring#263 (comment)
They are quire similar in Spring. It should be done in the same way.

Also it would be good (it's not clear whether this is feasible) to avoid using an additional qualifier for the scope owner: do it somehow in a way similar to Spring. But may be this is not possible.

@pleku pleku moved this from P1 - High priority to P2 - Medium Priority in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Sep 10, 2019
@kumm
Copy link
Contributor

kumm commented Sep 15, 2019

I have a simple POC to support current V14 behaviour.
The addon preserve ( by moving from the old UIScoped store to the new ) the route context for preserved route owners ( components in the preserved route chain).
This approach need an event fired by vaadin before moving the components to the new UI on reload.

As it said it is not what users expect, but at least it is in pair with current @PreserveOnReload.

If vaadin will support preserve on reload for the entire UI by moving all components to a new UI, then all UI scoped, and route scoped beans can be preserved same way. In fact it would be a little simpler.

It can be found here: kumm@f4d0910, kumm/flow@c7e66c3

@denis-anisimov
Copy link

see #369

@TatuLund TatuLund changed the title @PreserveOnRefresh doesn't preserve scoped beans @PreserveOnRefresh doesn't UI preserve scoped beans Jul 29, 2021
@TatuLund TatuLund changed the title @PreserveOnRefresh doesn't UI preserve scoped beans @PreserveOnRefresh doesn't preserve UI scoped beans Jul 29, 2021
@pleku
Copy link
Contributor

pleku commented Aug 10, 2021

Closing this as a won't fix, as the issue is addressed in https://github.com/vaadin/cdi/releases/tag/13.0.0.alpha1 (version for Vaadin 21) with changing the behavior of @RouteScoped.

We don't have plans at the moment for backporting the fix to the Vaadin 14 version of CDI add-on as it is a behavior change and could affect existing applications. If someone needs this badly for Vaadin 14, we could maybe make a new intermediate version where the feature is backported, and then users can opt-in to use it. Please comment here if that would be desired.

@pleku pleku closed this as completed Aug 10, 2021
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Product backlog to Done - pending release Aug 10, 2021
Vaadin Flow enhancements backlog (Vaadin 10+) automation moved this from Next for Dev. Team to Done / Pending Release Aug 10, 2021
@pleku pleku removed this from Done / Pending Release in Vaadin Flow enhancements backlog (Vaadin 10+) Aug 10, 2021
@pleku pleku removed this from Done - pending release in OLD Vaadin Flow ongoing work (Vaadin 10+) Aug 10, 2021
@pleku pleku added wontfix and removed blocked labels Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants