Leonardo - Thanks for this detailed analysis - lots of good information here. I've been meaning to follow up on this for some time now. Better late than never I guess. :-) My thoughts inline below... On 3/17/10 9:10 PM, Leonardo Uribe wrote: I have some concerns about the resource relocation implementation. As currently implemented (in Mojarra at least - haven't checked MyFaces yet), the <h:outputScript>/<h:outputStylesheet> components are literally removed from their original location in the component tree and are re-parented to the UIViewRoot. This has some non-obvious consequences, including: 1. EL no longer evaluates in its original context. 2. When re-executing the tags during render response on postbacks, the components are no longer found in their original location and are re-created (on every postback). #1 means that if an <h:outputScript>/<h:outputStylesheet> instance uses an EL expression that references context that is set up by an ancestor component, these EL expressions will fail to resolve correct after the component is relocated to a new parent. Mojarra contains code that attempts to work around this problem for one particular case: composite component context (ie. "#{cc}"). However, composite components are not the only components that may set up EL-reachable state. #2 means that we are doing unnecessary work - and potentially losing state since we re-create the component instance from scratch on each request. I *think* that the correct way to handle this is to leave the <h:outputScript>/<h:outputStylesheet> components in their original locations and to visit these before the render occurs to give these components a chance to contribute resources while in their proper context. This would allow ancestors to establish context that may be necessary for proper EL evaluation. You have hinted at something like this in the related "add component resources depending on the owner component state" thread (which I am also hoping to review) - ie. perhaps we need a new event/hook after tag execution but before render that we can use to collect the rendered resources. Note that if we are thinking about adding such an event, we need to consider doing this in a way that allows components to opt into event delivery. That is, instead of performing yet another full tree traversal, ideally we should perform a partial tree visit where we only visit components that have expressed an interest in performing pre-render processing. (Or, if no such components are present, we can skip the visit altogether.) This is going to be an important optimization for the use cases that I care the most about: views with very large/deep component trees. Oh, and, one nice side effect to moving resource collection out of PostAddToViewEvent and into a special pre-render processing pass is that we could take advantage of VisitHint.SKIP_UNRENDERED to ensure that we only visit/collect resources for components that are, in fact, rendered. Our current approach doesn't allow for this - ie. PostAddToViewEvents are delivered to both rendered and non-rendered components, and as a result all resources are added regardless of whether they are in rendered subtrees. I don't fully understand why the PostAddToViewEvents approach is necessary for implementing composite component facet/children insertion. The current solution - relocating composite component facets/children to a new parent inside of the composite component implementation suffers from problem #2 above. And in this case, the loss of state that occurs when re-creating these components is significant. Stateful components - such as Trinidad's <tr:showDetail>: http://myfaces.apache.org/trinidad/trinidad-api/tagdoc/tr_showDetail.html Cannot be used as a facet/child of a JSF 2 composite component, since any state that they maintain is lost after tags are re-executed on postback (during render response). Fortunately, we have a precedent for how to solve this problem - ie. Facelets already handles this nicely for the <ui:composition>/<ui:define>/<ui:insert> case, which is very similar to composite component facet/children insertion. Facelets uses the TemplateClient API to ensure that components are inserted into the proper target location *during* tag execution. When tags are re-executed against an existing component tree on postbacks, the components are still in the same location (in the target insertion location) and thus are not re-created. There is no loss of state. We are using a similar approach here to implement ADF Faces declarative components (our own pre-JSF2 composite component solution) and don't suffer from any state loss problems. (Note that we would use the TemplateClient API directly, but this was hidden in JSF2 so we ended up hacking around this to achieve the same behavior.) Is there a reason why the Facelets TemplateClient approach was not/cannot be used for composite components as well? This seems like a good use of PostAddToViewEvents. (Not sure that we really should be leveraging PostAddToViewEvents for the other two use cases.) Right - this is during restore view when we execute tags to build up the view. Note that once Mojarra issue 1313 is fixed: https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1313 We will also re-execute tags during render response (as we already do for full state saving), which means that we'll see additional PostAddToViewEvents. (This is the 1313 case - ie. we'll see this with partial state saving enabled too once 1313 is fixed.) Hrm. Something is very off here. I sort of get the idea that Facelets needs to detach/re-attach components to restore the original order. (Sort of. Does JSP do this too?). However, the fact that Facelets happens to temporarily remove and then immediately re-add components for the purpose of preserving the original order does not, in my mind, mean that we should be delivering PostAddToViewEvents for this scenario. The component was added to the component tree during restore view... it is still in the component tree, in the same parent, possibly in the same exact location under that parent after we re-execute the tags during render response. This does not strike me as a "component added" situation. The fact that Facelets happens to do this should be transparent to component authors - ie. we should not be delivering PostAddToViewEvents for this case. I can see how the current implementation can be very confusing. I agree that we need to simplify this. I believe that at least part of the problem is that we are delivering PostAddToViewEvents for cases that shouldn't be treated as an "add". This seems like an implementation bug to me, though perhaps some spec clarification is also necessary. I suspect that a post tag execution/pre-render (partial) visit might work better for this case as well. Is that because we are re-delivering PostAddToViewEvents during refresh for components which were already present in the component tree? If so, we should stop doing that. :-) Agreed. The fact that the developer needs to get creative here seems like a sign that something is off. Exactly! That's my #2 above. This definitely needs to be fixed. As I mentioned above, I think the right way to solve this problem is to use the Facelets TemplateClient approach, though I am interested to hear whether there are reasons why we did not/could not use this solution. I don't think that we should be delivering PostAddToViewEvents for these temporary detach/re-attach cases. I think that we should use a post tag execution/pre-render partial visit to collect these resources. I am very nervous about adding new events that are delivered to all components since this adds undesirable overhead for views with large #s of components. If we do need to add new events, I would prefer that we go with partial visits rather than full visits if at all possible. Yep, we definitely need some clarification here. Given the inconsistencies/unexpected behaviors in the current implementation, I can see how our users will find these events confusing. I don't see the distinction that you are making here... ie. "PostAddToView" and "PostAddToComponentTree" seem very similar to me. Andy