I think it should be an optional add-on (if included in the library at all), so that core/extensions components won't rely on it, and application developers may use it if they want the convenience. It is just a convenience, after all.
If it is enabled by default, it may cause really weird, hard-to-trace bugs in applications. If so, it will be hard to upgrade from previous versions (it's much, much worse than a method rename, because the IDE won't warn you in any way). And, what will I do if I don't want some IModel attribute to be detached? I declare an array of one element, just to 'hide' it from the detacher? Besides, AFAIK, Wicket's use of reflection has been pretty limited, and in very predictable forms. If at least you had to annotate these attributes to enable it, it wouldn't be so bad, but this feels like is magic. This is the kind of thing that makes Seam and CDI good for demos and awful for real applications. Convenient, saves two lines of code, and you never know what is going on. On Sun, Apr 8, 2012 at 12:51 PM, Igor Vaynberg <igor.vaynb...@gmail.com> wrote: > like i said, this only makes sense as a "core" features. because > components implemented with this enabled will not work properly in an > application where this is enabled. this is an all-or-nothing feature. > > -igor > > On Sun, Apr 8, 2012 at 3:15 AM, Johan Compagner <jcompag...@gmail.com> wrote: >> i think it would be fine to have something like this, and enabled by default >> but only to have an option to turn it off >> >> >> On Sat, Apr 7, 2012 at 22:30, Igor Vaynberg <igor.vaynb...@gmail.com> wrote: >> >>> -1 on adding it if its not enabled by default. its a trivial class >>> thats only about 40-50 lines of real code. adding it into extensions >>> and not using it will just add to code rot because i doubt many people >>> will go out looking for something like this since most of them wont >>> even know that its possible to do this. >>> >>> -igor >>> >>> On Fri, Apr 6, 2012 at 11:28 PM, Sven Meier <s...@meiers.net> wrote: >>> > The listener won't be set in IFrameworkSettings by default, right? >>> > IMHO it's better located in extensions then. >>> > >>> > Sven >>> > >>> > >>> > On 04/07/2012 01:37 AM, James Carman wrote: >>> >> >>> >> Add the listener to core and if folks want to use it they can. You >>> could >>> >> have a component instantiation listener add the detach listener to the >>> >> components. Another option would be an aspect. >>> >> On Apr 6, 2012 12:43 PM, "Igor Vaynberg"<igor.vaynb...@gmail.com> >>> wrote: >>> >> >>> >>> i wrote a IDetachListener that automatically detaches any IModel >>> >>> fields found on components. is this something we would be interested >>> >>> in for core? its been running in production for a while without any >>> >>> noticeable overhead and its nice not to have to implemenet onDetach() >>> >>> all the time just to forward it to secondary models. the only downside >>> >>> is that once we introduce this feature we can never remote it because >>> >>> doing so will break code. >>> >>> >>> >>> thoughts? >>> >>> >>> >>> -igor >>> >>> >>> > >>>
