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

Note on @CDIComponent with regard to Jakarte EE 10 #411

Open
roeltje25 opened this issue Oct 10, 2022 · 8 comments
Open

Note on @CDIComponent with regard to Jakarte EE 10 #411

roeltje25 opened this issue Oct 10, 2022 · 8 comments

Comments

@roeltje25
Copy link
Contributor

I refer to issue #303 where I addressed that @RouteScoped and @UIScoped annotations were properly taken into account when using bean-discovery-mode="annotated". The decision was taken to require a @CDIComponent annotation for any bean with @RouteScoped annotation when using bean-discovery-mode="annotated". Those that use this discovery mode will then have to use double annotations for all instances where @RouteScoped is used.

Now with that context in mind I notice that in Jakarte EE 10 the default bean-discovery-mode is changed to "annotated". Which will mean that the impact of the requirement of needing an @CDIComponent annotation is enlarged to any and all that do not explicitly have set their bean-discovery-mode to "all".

I do dare to stubbornly but confidently suggest to mark the @RouteScoped and @UIScoped annotations @stereotype after all to prevent users to code tautologies ( extends Component and also mark CDIComponent, and also say @RouteScoped) and prevent any future problems when people migrate to Jakarta EE 10.

@roeltje25
Copy link
Contributor Author

Anyone thinking about this issue?

@mshabarov
Copy link
Contributor

@roeltje25 thanks for the issue. Yes, we are thinking how we can fix it on our side and avoid extra annotations.

@mshabarov
Copy link
Contributor

The problem with bean discovery mode is a bit wider: without setting it to all Vaadin applications end up with views not discovered as CDI beans even if they have no scope annotations on top of it, see for instance this issue with Payara server vaadin/flow#15625 (comment)

@roeltje25
Copy link
Contributor Author

I know. This is the exact "problem" or in fact feature we have dealt with all along. We always explicitly set the bean discovery mode to annotated in our projects. To us that makes more sense (performance and control). We annotate all our view with @RouteScoped or @UIScoped. Which in fact gives issues if they are not marked as @stereotype. In CDI with annotated mode I would consider it a bug if a bean is discovered while not explicitly having a bean defining annotation, so @route may not be the right candidate to become a bean defining annotation. But the Scopes suggest to be scopes, but they are not implemented as bean defining annotations, while as far as my knowledge goes, all other scopes (in CDI for sure, also the scopes in the projects I know) are bean defining as well..... except for the @RouteScoped and @UIScoped in Vaadin. This is the exact reason for my pushing for those annotations to be bean defining annotations.

@mcollovati
Copy link
Contributor

mcollovati commented Jan 31, 2023

I may be wrong but in CDI only @ApplicationScoped, @RequestScoped, @Dependent and normal scopes are bean defining annotations

https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#bean_defining_annotations

And, if I recall correctly, we have normal route and ui scope.

Of course they behave differently from the non normal scope, and cannot be used on vaadin components

I am still not convinced of annotating the non normal scopes with stereotype, but this is just my personal opinion

@roeltje25
Copy link
Contributor Author

roeltje25 commented Jan 31, 2023 via email

@mcollovati
Copy link
Contributor

I know that adding a stereotype makes an annotations a bean defining one. My point is that by spec only some kinds of scopes are naturally bean defining annotations and that specs says that a stereotype may have a scope but there is no mention about making a scope a stereotype.
But, again, that is only my point of view.
The team will decided how to handle this

@mshabarov
Copy link
Contributor

I personally don't have enough expertise to decide to include AtStereotype or not.
This is not a priority at the moment for Flow team.
Let us keep this in a backlog until further investigation of this topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🔖 Normal Priority (P2)
Development

No branches or pull requests

3 participants