I'm sorry for the late reply, we are in the middle of a migration from svn to 
git and that's taking most of my time at the moment.

Igor:
I already renamed the classes as you suggested, added a warning when the 
minified version cannot be found and changed the file-extension detection.

I tried to rewrite both resource references as a wrapper around 
PackageResourceReference, but that proved quite difficult. You somehow have to 
intercept the call to getName() in the resource reference itself, because its 
value gets passed to the resource directly. The only solutions I see are:
 - wrapping the resource, which still requires quite some changes to the 
resource
 - adding a second getResource to PackageResourceReference method that takes 
all values as parameters
 - adding a setName to PackageResourceReferences
I do not like any of these solutions, so I will leave it for now and see if I 
can come up with something better later on. Whatever solution is chosen, it 
needs to prevent duplicate resources that can result from adding a minified 
and a full version, this is not solved yet.

The option to disable the use of minified resources commes from WiQuery. We 
don't use it, but perhaps other do. I don't care if it is removed.

Igor and Peter:
Don't you think preventing infinity loops is the resposibillity of the 
developer? After all, the developer declares the dependencies. The check can 
become quite costly, because you have to search in the list of 
RecordedHeaderItem.recordedLocations or keep a list of all added dependencies 
while adding them. I haven't tried catching the StackOverflowError that you 
would get, perhaps that would work? Another option would be to limit the depth 
of the dependencies to 100 for example and throw an exception at 100 recursive 
iterations.

I do like the idea of rewriting IHeaderResponse to use HeaderItem, or 
IHeaderItem or whatever works best. However, that would be a rather big 
refactoring which I'd rather do after introducing the real features. Also, I'm 
not sure if this would read very well:
  
response.render(JavaScriptHeaderItem.forReference(JQueryResourceReference.get()));
It's a bit more verbose than:
  response.renderJavaScriptReference(JQueryResourceReference.get());

Peter:
The wasRendered and markRendered are part of the public API of 
IHeaderResponse. They are also used by Component and Behavior to track 
rendering of resources. If these methods are changed, that part of the 
rendering needs to be changed as well and we do need some replacement, because 
I (mis)use then to know when rendering for a certain component starts and 
ends.

Best regards,
Emond

On Monday 05 December 2011 14:25:59 Peter Ertl wrote:
> maybe we could get rid of some 'generic' declarations and replace
> 
> IHeaderResponse#wasRendered(Object) with
> IHeaderResponse#wasRendered(IHeaderItem)
> IHeaderResponse#markRendered(Object) with
> IHeaderResponse#markRendered(IHeaderItem)
> 
> IHeaderResponse#renderString(CharSequence) with
> IHeaderResponse#renderString(StringHeaderItem)   [or just use a generic
> render(IHeaderItem) like Igor suggested)
> Am 05.12.2011 um 14:18 schrieb Peter Ertl:
> > my 2%
> > 
> > - introduce interface IHeaderItem (next to abstract class HeaderItem)
> > and use that where possible so migration of existing resources will be
> > easier - rename render(IHeaderResponse) to renderHead(IHeaderResponse)
> > to avoid confusion since 'render(response)' looks to much like it
> > renders all, not just the head. - what happens if you have cyclic
> > dependencies?
> > 
> > Am 03.12.2011 um 17:17 schrieb Igor Vaynberg:
> >> also since we now have HeaderItem and its used in the core part of the
> >> framework (ResourceReference) why not refactor IHeaderResponse to just
> >> have one render(HeaderItem) instead of the ton of methods in there
> >> now...
> >> 
> >> -igor
> >> 
> >> On Sat, Dec 3, 2011 at 7:29 AM, Igor Vaynberg <[email protected]> 
wrote:
> >>> looks really good. here are some notes:
> >>> 
> >>> * MinifiedDetecting*->MinifiedAware*
> >>> * MinifiedDetecting*#getName() needs to have a code comment saying
> >>> that the code inside has to be threadsafe so when people tweak it
> >>> they
> >>> are aware of it
> >>> * can the MinifiedDetecting* be made into a wrapper instead of
> >>> different subclasses
> >>> * MinifiedDetecting* doesnt check for the actual file extension, it
> >>> assumes that its either js or css
> >>> * MinifiedDetecting* should warn if minified resource is missing
> >>> 
> >>> * ResourceAggrator -> ResourceAggrator
> >>> * ResourceAggrator#renderResources() make sure this is
> >>> infinite-recursion-proof if two dependencies reference each other
> >>> * i dont think getResourceSettings().getUseMinifiedResources() is
> >>> necessary, just use application's developmentmode flag
> >>> 
> >>> -igor
> >>> 
> >>> On Fri, Dec 2, 2011 at 12:33 AM, Emond Papegaaij
> >>> 
> >>> <[email protected]> wrote:
> >>>> Hi all,
> >>>> 
> >>>> For the past few weeks, and especially the last few days, Hielke
> >>>> Hoeve and I have been working on improvements to resource
> >>>> management in Wicket. Most of the improvements are based on work
> >>>> in WiQuery, but the actual implementation is from scratch. The
> >>>> targets for the improvements can be found in WICKET-4273. In
> >>>> short, it boils down to following points:
> >>>> - Dependency support for resources
> >>>> - Sorting of resources in the header
> >>>> - Native resource bundle support in Wicket
> >>>> - Aggregating many small scripts into 1 large script tag, esp. for
> >>>> events
> >>>> 
> >>>> The target for these changes will be Wicket 6 and the work in
> >>>> progress can be found on github:
> >>>> https://github.com/papegaaij/wicket/compare/trunk...wicket%2Bwique
> >>>> ry
> >>>> 
> >>>> At the moment, all features, except the resource bundles are
> >>>> implemented and working. Documentation is still missing on most
> >>>> places. I've also not yet come to writing tests and an example on
> >>>> how to use it.
> >>>> 
> >>>> Please provide your feedback on the code, here on the mailing list
> >>>> or at JIRA.
> >>>> 
> >>>> Note to Jeremy: I deleted some of the code you contributed to
> >>>> Wicket 1.5 because there was a large overlap in functionality,
> >>>> and it proved difficult to keep the old code working as is. It
> >>>> would be great if you could shed some light on what the exact
> >>>> problem was, you were trying to solve with that code, so I can
> >>>> make sure that it can also be solved with this new approach.
> >>>> 
> >>>> Best regards,
> >>>> Emond Papegaaij

Reply via email to