Closed anaili closed 8 years ago
@yesbutno you might be concerned by this PR. As you worked on StaticJsCache.
I'm not sure I completely understand how this works and would really appreciate some help to figure things out:
StaticJsCache
, if I call one of the append
methods after compress()
the *_SCRIPT
contains the uncompresses str
.*_SCRIPT
s are publicly r/w-accessible.DefaultStaticJsCacheLoader
seems to have nothing to do with the cache at all.StaticJsLoader
and I can't think of at least one more meaningful subclass - so why the additional abstract base class?StaticJsCacheFactory
to be usable BuildStaticJsCache()
must be called - why not move the code directly into the constructor?StaticJsCacheFactory
all three methods have to be called in the exact order - wouldn't it be better to move the calls into an init()
method in StaticJsLoader
?Sorry for the lot of questions, but maybe you could help me out.
No problem, feel free to ask any questions !
DefaultStaticJsCacheLoader
is the class responsible of filling up these fields in StaticJsCache
( I don't know why did you say it has nothing to do with the cache).BuildStaticJsCache
may throws exceptions, uses internally reflections, allocate ressources, ... so I don't think it's good by design to put it in the constructor.StaticJsCacheFactory
: correct, I had to apply a template pattern in StaticJsLoader
to force calling order.
I'm still working on that anyway.Thanks! And now me again ;-)
append() / compress()
: Nice. And then something like IllegalStateException
when calling append()
after compressing?DefaultStaticJsCacheLoader
: My mistake. I meant getAppName()
inside DefaultStaticJsCacheLoader
.BuildStaticJsCache
: Agreed. Currently the exceptions are, well, "handled" inside the method. Is there a use case where it's not necessary to call the method? My point is that it has to be impossible to get an uninitialized factory. Actually, getAppName()
returns the ngApp attribute's value which is mandatory in script generation because it has to be injected in 'app' module. The question now is, why we don't let the user implement the @NGApp
annotated class ? Maybe @bessemHmidi has an explanation for this. About BuildStaticJsCache
, I'm working on a different way to instantiate the StaticJsCacheLoader
.
Oh dear. I see there's still one thing or two I can learn about the actual meaning of the codebase ;-)
I only saw @Named
being the only annotation that is handled in getAppName()
and no access to any parts of this static js cache, and the name actually coming from the BeanRegistry
...
i will explain anout @NGApp:
i wanted in the begining to let the user declare his app (or apps), and if not provided angularBean will provide a default one.
but after that i remarqued that with the multiple apps we need a mapping or a grouping of the different angularBeans to their app, for example something like @AngularBean(appName='app1')
and in the js side providing different js import : script src='angular-beans-core'> script src='app1.js'> script src='app2.js'>
all that need a small study to be efficient and even i thinked about AMD or asynchronous js loading, that is important for a big application. but Angular2 provide a built in dependency mecanisme so, i preferred to move the effort to that version.
Hope that was clear :)
@yesbutno completely undrestandable :) and sorry for the lack of code comments.
Created a factory pattern for static javascript cache generation with some code cleanup and optimization.