You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Pedro Santos <pe...@gmail.com> on 2016/08/20 04:02:04 UTC
Container rendering change proposal
Hi devs,
Wicket's container rendering creates a new component for each
wicket:fragment it finds in the markup, and puts it in the component tree.
This process has a few issues and I prose us to simplify it.
issues:
- the new component for the fragment can conflict with user's components
since we need a wicket:id for it and we don't have a reserved namespace
- can cause unexpected behaviours like a container with no children testing
true for container.size() > 0
- adds an unnecessary object in the tree
- adds unnecessary complexity like custom component versioning (we are
setting this component to be not versioned)
- causes misleading exception messages since the fragment markup can be
mistaken by an actual component markup
My idea it to simple skip wicket:fragment's markup while rendering
containers. The only place we need to load this markup inside the markup
sourcing strategy when providing for a Fragment.
The implications are to remove FragmentResolver and possible to change
wicket:fragment tag's id attribute from wicket:id to fragment-id or id in
Wicket 8.
I see this as a non trivial internal change for Wicket 6 and 7, so I worked
on a branch[1] to showcase the idea and to get your thoughts while
resolving WICKET-6219 in wicket-7.x branch.
cheers,
Pedro Santos
1 - https://github.com/apache/wicket/tree/WICKET-6219-no-fragment-resolver
Re: Container rendering change proposal
Posted by Andrea Del Bene <an...@gmail.com>.
Thank you Pedro! Everything that avoids redundant code is welcome! I
will also try to test the branch but I'm not sure I have an app with a
huge amount of fragments.
On 20/08/2016 13:12, Ernesto Reinaldo Barreiro wrote:
> I also have a customer that make an abundant use of fragments on their main
> application. I helped them migrate to wicket 7. I might be able to provide
> feedback as well on that area.
>
> On Sat, Aug 20, 2016 at 12:50 PM, Martin Grigorov <mg...@apache.org>
> wrote:
>
>> Hi Pedro,
>>
>> I won't be able to review and test this change in the next few days.
>> We use 7.4.0 in our main app and we have 22 usages of <wicket:fragment>.
>> Should be enough to validate the changes.
>> I'll let you know when I'm done!
>>
>> Martin Grigorov
>> Wicket Training and Consulting
>> https://twitter.com/mtgrigorov
>>
>> On Sat, Aug 20, 2016 at 6:02 AM, Pedro Santos <pe...@gmail.com> wrote:
>>
>>> Hi devs,
>>>
>>> Wicket's container rendering creates a new component for each
>>> wicket:fragment it finds in the markup, and puts it in the component
>> tree.
>>> This process has a few issues and I prose us to simplify it.
>>>
>>> issues:
>>>
>>> - the new component for the fragment can conflict with user's components
>>> since we need a wicket:id for it and we don't have a reserved namespace
>>> - can cause unexpected behaviours like a container with no children
>> testing
>>> true for container.size() > 0
>>> - adds an unnecessary object in the tree
>>> - adds unnecessary complexity like custom component versioning (we are
>>> setting this component to be not versioned)
>>> - causes misleading exception messages since the fragment markup can be
>>> mistaken by an actual component markup
>>>
>>> My idea it to simple skip wicket:fragment's markup while rendering
>>> containers. The only place we need to load this markup inside the markup
>>> sourcing strategy when providing for a Fragment.
>>>
>>> The implications are to remove FragmentResolver and possible to change
>>> wicket:fragment tag's id attribute from wicket:id to fragment-id or id in
>>> Wicket 8.
>>>
>>> I see this as a non trivial internal change for Wicket 6 and 7, so I
>> worked
>>> on a branch[1] to showcase the idea and to get your thoughts while
>>> resolving WICKET-6219 in wicket-7.x branch.
>>>
>>> cheers,
>>>
>>> Pedro Santos
>>>
>>> 1 - https://github.com/apache/wicket/tree/WICKET-6219-no-
>> fragment-resolver
>
>
Re: Container rendering change proposal
Posted by Ernesto Reinaldo Barreiro <re...@gmail.com>.
I also have a customer that make an abundant use of fragments on their main
application. I helped them migrate to wicket 7. I might be able to provide
feedback as well on that area.
On Sat, Aug 20, 2016 at 12:50 PM, Martin Grigorov <mg...@apache.org>
wrote:
> Hi Pedro,
>
> I won't be able to review and test this change in the next few days.
> We use 7.4.0 in our main app and we have 22 usages of <wicket:fragment>.
> Should be enough to validate the changes.
> I'll let you know when I'm done!
>
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
>
> On Sat, Aug 20, 2016 at 6:02 AM, Pedro Santos <pe...@gmail.com> wrote:
>
> > Hi devs,
> >
> > Wicket's container rendering creates a new component for each
> > wicket:fragment it finds in the markup, and puts it in the component
> tree.
> > This process has a few issues and I prose us to simplify it.
> >
> > issues:
> >
> > - the new component for the fragment can conflict with user's components
> > since we need a wicket:id for it and we don't have a reserved namespace
> > - can cause unexpected behaviours like a container with no children
> testing
> > true for container.size() > 0
> > - adds an unnecessary object in the tree
> > - adds unnecessary complexity like custom component versioning (we are
> > setting this component to be not versioned)
> > - causes misleading exception messages since the fragment markup can be
> > mistaken by an actual component markup
> >
> > My idea it to simple skip wicket:fragment's markup while rendering
> > containers. The only place we need to load this markup inside the markup
> > sourcing strategy when providing for a Fragment.
> >
> > The implications are to remove FragmentResolver and possible to change
> > wicket:fragment tag's id attribute from wicket:id to fragment-id or id in
> > Wicket 8.
> >
> > I see this as a non trivial internal change for Wicket 6 and 7, so I
> worked
> > on a branch[1] to showcase the idea and to get your thoughts while
> > resolving WICKET-6219 in wicket-7.x branch.
> >
> > cheers,
> >
> > Pedro Santos
> >
> > 1 - https://github.com/apache/wicket/tree/WICKET-6219-no-
> fragment-resolver
> >
>
--
Regards - Ernesto Reinaldo Barreiro
Re: Container rendering change proposal
Posted by Pedro Santos <pe...@gmail.com>.
Martijn, thanks! I put my thoughts on the id change in a different thread.
Giving it a closer look, I no longer see a relation with the rendering
change.
Martin, Ernesto, Andrea, thanks! All tests are welcome. Really happy that
it worked fine there, Martin.
Cheers,
Pedro Santos
Pedro Santos
On Sat, Aug 27, 2016 at 3:16 PM, Martin Grigorov <mg...@apache.org>
wrote:
> Hi Pedro,
>
> I've tested our main application against your branch and everything looks
> OK!
> The diff also looks good!
>
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
>
> On Sat, Aug 20, 2016 at 12:50 PM, Martin Grigorov <mg...@apache.org>
> wrote:
>
> > Hi Pedro,
> >
> > I won't be able to review and test this change in the next few days.
> > We use 7.4.0 in our main app and we have 22 usages of <wicket:fragment>.
> > Should be enough to validate the changes.
> > I'll let you know when I'm done!
> >
> > Martin Grigorov
> > Wicket Training and Consulting
> > https://twitter.com/mtgrigorov
> >
> > On Sat, Aug 20, 2016 at 6:02 AM, Pedro Santos <pe...@gmail.com>
> wrote:
> >
> >> Hi devs,
> >>
> >> Wicket's container rendering creates a new component for each
> >> wicket:fragment it finds in the markup, and puts it in the component
> tree.
> >> This process has a few issues and I prose us to simplify it.
> >>
> >> issues:
> >>
> >> - the new component for the fragment can conflict with user's components
> >> since we need a wicket:id for it and we don't have a reserved namespace
> >> - can cause unexpected behaviours like a container with no children
> >> testing
> >> true for container.size() > 0
> >> - adds an unnecessary object in the tree
> >> - adds unnecessary complexity like custom component versioning (we are
> >> setting this component to be not versioned)
> >> - causes misleading exception messages since the fragment markup can be
> >> mistaken by an actual component markup
> >>
> >> My idea it to simple skip wicket:fragment's markup while rendering
> >> containers. The only place we need to load this markup inside the markup
> >> sourcing strategy when providing for a Fragment.
> >>
> >> The implications are to remove FragmentResolver and possible to change
> >> wicket:fragment tag's id attribute from wicket:id to fragment-id or id
> in
> >> Wicket 8.
> >>
> >> I see this as a non trivial internal change for Wicket 6 and 7, so I
> >> worked
> >> on a branch[1] to showcase the idea and to get your thoughts while
> >> resolving WICKET-6219 in wicket-7.x branch.
> >>
> >> cheers,
> >>
> >> Pedro Santos
> >>
> >> 1 - https://github.com/apache/wicket/tree/WICKET-6219-no-fragmen
> >> t-resolver
> >>
> >
> >
>
Re: Container rendering change proposal
Posted by Martin Grigorov <mg...@apache.org>.
Hi Pedro,
I've tested our main application against your branch and everything looks
OK!
The diff also looks good!
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Sat, Aug 20, 2016 at 12:50 PM, Martin Grigorov <mg...@apache.org>
wrote:
> Hi Pedro,
>
> I won't be able to review and test this change in the next few days.
> We use 7.4.0 in our main app and we have 22 usages of <wicket:fragment>.
> Should be enough to validate the changes.
> I'll let you know when I'm done!
>
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
>
> On Sat, Aug 20, 2016 at 6:02 AM, Pedro Santos <pe...@gmail.com> wrote:
>
>> Hi devs,
>>
>> Wicket's container rendering creates a new component for each
>> wicket:fragment it finds in the markup, and puts it in the component tree.
>> This process has a few issues and I prose us to simplify it.
>>
>> issues:
>>
>> - the new component for the fragment can conflict with user's components
>> since we need a wicket:id for it and we don't have a reserved namespace
>> - can cause unexpected behaviours like a container with no children
>> testing
>> true for container.size() > 0
>> - adds an unnecessary object in the tree
>> - adds unnecessary complexity like custom component versioning (we are
>> setting this component to be not versioned)
>> - causes misleading exception messages since the fragment markup can be
>> mistaken by an actual component markup
>>
>> My idea it to simple skip wicket:fragment's markup while rendering
>> containers. The only place we need to load this markup inside the markup
>> sourcing strategy when providing for a Fragment.
>>
>> The implications are to remove FragmentResolver and possible to change
>> wicket:fragment tag's id attribute from wicket:id to fragment-id or id in
>> Wicket 8.
>>
>> I see this as a non trivial internal change for Wicket 6 and 7, so I
>> worked
>> on a branch[1] to showcase the idea and to get your thoughts while
>> resolving WICKET-6219 in wicket-7.x branch.
>>
>> cheers,
>>
>> Pedro Santos
>>
>> 1 - https://github.com/apache/wicket/tree/WICKET-6219-no-fragmen
>> t-resolver
>>
>
>
Re: Container rendering change proposal
Posted by Martin Grigorov <mg...@apache.org>.
Hi Pedro,
I won't be able to review and test this change in the next few days.
We use 7.4.0 in our main app and we have 22 usages of <wicket:fragment>.
Should be enough to validate the changes.
I'll let you know when I'm done!
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Sat, Aug 20, 2016 at 6:02 AM, Pedro Santos <pe...@gmail.com> wrote:
> Hi devs,
>
> Wicket's container rendering creates a new component for each
> wicket:fragment it finds in the markup, and puts it in the component tree.
> This process has a few issues and I prose us to simplify it.
>
> issues:
>
> - the new component for the fragment can conflict with user's components
> since we need a wicket:id for it and we don't have a reserved namespace
> - can cause unexpected behaviours like a container with no children testing
> true for container.size() > 0
> - adds an unnecessary object in the tree
> - adds unnecessary complexity like custom component versioning (we are
> setting this component to be not versioned)
> - causes misleading exception messages since the fragment markup can be
> mistaken by an actual component markup
>
> My idea it to simple skip wicket:fragment's markup while rendering
> containers. The only place we need to load this markup inside the markup
> sourcing strategy when providing for a Fragment.
>
> The implications are to remove FragmentResolver and possible to change
> wicket:fragment tag's id attribute from wicket:id to fragment-id or id in
> Wicket 8.
>
> I see this as a non trivial internal change for Wicket 6 and 7, so I worked
> on a branch[1] to showcase the idea and to get your thoughts while
> resolving WICKET-6219 in wicket-7.x branch.
>
> cheers,
>
> Pedro Santos
>
> 1 - https://github.com/apache/wicket/tree/WICKET-6219-no-fragment-resolver
>
Re: Container rendering change proposal
Posted by Martijn Dashorst <ma...@gmail.com>.
Ehm...
WOW?
Nice comeback!
I doubt that our company's applications would be a good test for this,
as our exposure to fragments is limited (I always found them too
cumbersome to use, and always opt for panels). I'd hope that someone
with a fancy for fragments would test this in their application(s).
In any case, we can add it to 8 and then it's available in m2. My
guess is that it is not hard to reverse if it were to cause
insurmountable problems (just a minor pain when folks need to revert
the id change).
Martijn
On Sat, Aug 20, 2016 at 6:02 AM, Pedro Santos <pe...@gmail.com> wrote:
> Hi devs,
>
> Wicket's container rendering creates a new component for each
> wicket:fragment it finds in the markup, and puts it in the component tree.
> This process has a few issues and I prose us to simplify it.
>
> issues:
>
> - the new component for the fragment can conflict with user's components
> since we need a wicket:id for it and we don't have a reserved namespace
> - can cause unexpected behaviours like a container with no children testing
> true for container.size() > 0
> - adds an unnecessary object in the tree
> - adds unnecessary complexity like custom component versioning (we are
> setting this component to be not versioned)
> - causes misleading exception messages since the fragment markup can be
> mistaken by an actual component markup
>
> My idea it to simple skip wicket:fragment's markup while rendering
> containers. The only place we need to load this markup inside the markup
> sourcing strategy when providing for a Fragment.
>
> The implications are to remove FragmentResolver and possible to change
> wicket:fragment tag's id attribute from wicket:id to fragment-id or id in
> Wicket 8.
>
> I see this as a non trivial internal change for Wicket 6 and 7, so I worked
> on a branch[1] to showcase the idea and to get your thoughts while
> resolving WICKET-6219 in wicket-7.x branch.
>
> cheers,
>
> Pedro Santos
>
> 1 - https://github.com/apache/wicket/tree/WICKET-6219-no-fragment-resolver
--
Become a Wicket expert, learn from the best: http://wicketinaction.com