StateHelper: make methods generics aware
like
public Object eval(Serializable key, Object defaultValue);
to
public <T> T eval(Serializable key, T defaultValue);
https://github.com/apache/myfaces/commit/a58cb8c3a731ff5efe9a5e7f8523aacd02eeb41b
Where is the discussion? And what are the downsides?
Do we need a discussion for it? IMO it just makes the API generics aware, which is done like 10 years to late.
+1
+1
at the moment the only solution is to use the OmniFaces State wrapper!
I can't generify Object eval(Serializable key, Supplier<Object> defaultValueSupplier); as this makes existing impls incompatible. E.g. ComponentStateHelper doesn't anymore compile after this change. This thus breaks backwards compatibility. The correct procedure is to add a new method (with different name) and deprecate the old method with a (javadoc) reference to the new method.
Can you perhaps propose a suitable name for the new method or a working/acceptable alternative?
Imo we can just break it? No applications use a custom one and its a Migration to 5.0
Yeah, but it's a public API and therefore the JEE spec committee won't accept this .. The correct procedure is deprecation.
Unless there's an alternative I'm not aware of, @arjantijms?
AFAIR we have already breaking changes like making FacesMessage.Severity to enum? if a user weirdly uses this, it might break already
AFAIR we have already breaking changes like making FacesMessage.Severity to enum? if a user weirdly uses this, it might break already
The enum usage is 100% backwards compatible with existing code syntax for class constants. And, incompatible methods have been marked deprecated for removal with a reference to the new/correct approach.
hm i see
i just would like to add that i maintain libs and projects around JSF since 15 years and never seen a custom StateHelper
and it would be a shame to using a different name just because we introduce generics like 10 years to late.
Fully agree.
Hence, @arjantijms do you know a backdoor which would allow us making this compile-time incompatible change? Or perhaps @mnriem?
To summarize, we'd like to change in StateHelper
Object eval(Serializable key, Supplier<Object> defaultValueSupplier);
to
<T> T eval(Serializable key, Supplier<T> defaultValueSupplier);
but then existing impls won't compile anymore and developer has to edit these.
I don't get the problem ...
if a developer upgrade to Faces 5 and in a custom component there is a getter like this:
public String getLocale() { return (String) getStateHelper().eval(PropertyKeys.locale); }
the fact that now the StateHelper returns a String do not create any compilation problem...
the cast become superfluos and it could be removed
The "jakarta faces end user" usage will be unaffected indeed. But we're talking about the implementation of the StateHelper interface. I.e. the actual code of whatever instance is returned by getStateHelper() method. This could have been customized (although super rare but not impossible). And this change would break it. This existing custom implementation would not anymore compile and would need editing. And this goes against the Jakarta EE spec rules wrt backwards compatibility.
isnt this just a theoretical problem? PrimeFaces and OmniFaces doesnt have it, every other lib isnt maintained for years?!
IMHO ... just talking ...
probably they are 2 o 3 devs in the world...
they could stay in 4.1 line and when they are ready to upgrade probably with some minutes of work they can improve their code...
I fully agree but I don't control JEE spec rules.
According to the current JEE spec rules, such change would block the inclusion of Faces 5.0 in JEE 12.
cc: @arjantijms -- is there really no way we can control/force this..??
I know, I know... ;)
Some of these "rules" are probably very outdated because the software industry moves 10 times faster today than it did 15 years ago.
Also the IDEs are very powerful and there is github ... it's a different story...
Implemented except for Object eval(Serializable key, Supplier<Object> defaultValueSupplier) due to backwards compatibility.
Reopening because backwards incompatibility is no issue for major version (as long as it's in changelog) and we now have control over old-TCK.