[
https://issues.apache.org/jira/browse/WICKET-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003032#comment-13003032
]
Maarten Billemont edited comment on WICKET-3218 at 3/5/11 7:47 PM:
-------------------------------------------------------------------
The solution is really simple. Stop with the inconsistencies, stop with
allowing constructor-created components everywhere, always. That's the whole
reason onInitialize was introduced.
This issue deals with the fact that things go wrong when onInitialize isn't
used properly. And instead of making your users do the right thing, you break
your API consistency, ludicrous.
put an assert in add() that forbids its usage during construction. You never
want people adding components to their page during construction. Ever. Teach
your userbase how to do it right, and make it part of the "upgrade-to-1.5"
learning curve.
Pages should behave the same way as any component as far as component hierarchy
construction is concerned, there's absolutely no sense to confusing your users
and breaking that consistency.
Re: your example:
EditUserPage page=new EditUserPage(user); page.switchToAddressEditMode();
switchToAddressEditMode() shouldn't be doing anything at all with components.
None of your methods should. Your component hierarchy should be constructed
ONCE in onInitialize, and remain STATIC for the rest of your component's
lifecycle. Everything that you want to do with regards to modifying how and
what your component renders should be toggled with models or state that gets
checked in onConfigure (with the rare exception of sometimes replacing a
component in onConfigure/onBeforeRender).
If you want to protect your users from themselves and not sacrifice your API to
suit the idiots, then tell them they're doing it wrong with a runtime exception.
was (Author: lhunath):
The solution is really simple. Stop with the inconsistencies, stop with
allowing constructor-created components everywhere, always. That's the whole
reason onInitialize was introduced.
This issue deals with the fact that things go wrong when onInitialize isn't
used properly. And instead of making your users do the right thing, you break
your API consistency, ludicrous.
put an assert in add() that forbids its usage during construction. You never
want people adding components to their page during construction. Ever. Teach
your userbase how to do it right, and make it part of the "upgrade-to-1.5"
learning curve.
Pages should behave the same way as any component as far as component hierarchy
construction is concerned, there's absolutely no sense to confusing your users
and breaking that consistency.
Re: your example:
EditUserPage page=new EditUserPage(user); page.switchToAddressEditMode();
switchToAddressEditMode() shouldn't be doing anything at all with components.
None of your methods should. Your component hierarchy should be constructed
ONCE in onInitialize, and remain STATIC for the rest of your component's
lifecycle. Everything that you want to do with regards to modifying how and
what your component renders should be toggled with models or state that gets
checked in onConfigure.
If you want to protect your users from themselves and not sacrifice your API to
suit the idiots, then tell them they're doing it wrong with a runtime exception.
> Component#onInitialize is broken for Pages
> ------------------------------------------
>
> Key: WICKET-3218
> URL: https://issues.apache.org/jira/browse/WICKET-3218
> Project: Wicket
> Issue Type: Bug
> Components: wicket-core
> Affects Versions: 1.4.14
> Reporter: Carl-Eric Menzel
> Assignee: Igor Vaynberg
> Fix For: 1.5-RC1
>
> Attachments:
> 0001-delay-oninitialize-until-just-before-onconfigure.patch
>
>
> As first mentioned at
> http://mail-archives.apache.org/mod_mbox/wicket-dev/201012.mbox/%3c1291238699.1749.1408166...@webmail.messagingengine.com%3E
> , I think the current (Wicket 1.4.14) implementation of
> Component#onInitialize is broken for Pages. Pages get initialized as soon as
> the first component is added, which is correct. But this usually happens
> within the constructor of the page, which means that the page object isn't
> fully initialized yet. The entire point of having onInitialize, however, is
> to be able to do further work once all constructors have run. See
> https://github.com/duesenklipper/wicket-oninitialize for a quickstart that
> demonstrates the problem.
> Pedro Santos suggested in the above thread to just switch the entire object
> construction to onInitialize. I don't think this is a good idea, because
> 1) it is completely counter-intuitive
> 2) it is not always realistic to have an entire class hierarchy not using the
> constructor just because a subclass somewhere might want to use onInitialize
> 3) it is inconsistent with onInitialize behavior for all other (non-Page)
> components. Here I can easily mix work in the constructor with onInitialize.
> I propose the following patch:
> - override onInitialize in Page and make it final, so Pages can't use this
> any more. This should not cause any unnecessary breaking, since currently
> it's not working for pages anyway.
> - introduce Page#onPageInitialize to provide a safe alternative to
> onInitialize
> - make a special case for Page in Component's beforeRender to fire
> Page#onPageInitialize if necessary
> Yes, this is a bit of special casing for Page, but there's quite a lot of
> that needed for Page anyway. I think the impact of this should be minimal.
> My page includes documentation and a new testcase that verifies the new
> behavior. I modified the old ComponentInitializationTest to reflect the fact
> that Page doesn't get onInitialize any more.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
