On Jan 31, 8:15 pm, Attila Szegedi <[email protected]> wrote: > Hi all, > > I just put out the second implementation patch at > <https://bugzilla.mozilla.org/show_bug.cgi?id=540724>, against current CVS > HEAD. Check it out if you're interested. I have worked on this for I most of > my free time I can have for coding in the last 12 days, and have arrived at a > much better design than what was in the first attempt. I attached a comment > to the Bugzilla issue describing the design decisions I took. It all feels > round to me at the moment. I took care to document all classes and interfaces > in great detail. I'll proceed with writing tests for it, but if you don't > mind reviewing and trying bleeding-edge stuff, go for it.
Hi Attila, I just had a look at your patch, here are some quick impressions. The general code layout seems clear and well thought out. The only problem I had is the similarity between ModuleSource and ModuleScript and related interfaces and classes. *ModuleSourceProvider and *ModuleScriptProvder just look very similar, and it doesn't help that both live in the same package. I know the naming makes sense, but maybe it's possible to find terms that are farther apart. In Helma NG, we use the terms Repository and Resource for module source interfaces. On the source provider side, I'm not sure the default implementation should be based on URI/URLs. I think local, file based module repositories will be more common than HTTP for CommonJS Rhino, and while URLs cover all this, they just don't have an easy validation mechanism for local files. At least I think there should be a FileModuleSourceProvider in addition to the UrlModuleSourceProvider. Other than that, I really like your implementation. Helma NG does a few things differently, so it's not likely we'll replace our implementation anytime soon, but I think I'll take some inspiration from your code for things it does better than ours. Hannes > Attila. > > On 2010.01.19., at 23:45, Attila Szegedi wrote: > > > Hi, > > > I created a Bugzilla issue to track this work: > > <https://bugzilla.mozilla.org/show_bug.cgi?id=540724> > > I already attached a patch with the implementation to the above issue, so > > feel free to check it out and provide feedback. I believe that the JavaDocs > > I provided are sufficiently comprehensive so that no one should have > > trouble understanding how's it used. > > > Be warned it's quite untested - I'll proceed with writing some tests > > tomorrow. > > > Attila. > > > -- > > home:http://www.szegedi.org > > twitter:http://twitter.com/szegedi > > weblog:http://constc.blogspot.com > > > On 2010.01.18., at 14:54, Jaros³aw Pa³ka wrote: > > >> Count me in as well. > > >> Jarek > > >> 2010/1/18 Rapha <[email protected]> > > >>> I like the idea. Are you thinking of the 1.1 spec ( > >>>http://wiki.commonjs.org/wiki/Modules/1.1) ? > > >>> Raphael > > >>> On Jan 17, 1:35 pm, Attila Szegedi <[email protected]> wrote: > >>>> Folks, > > >>>> I'm contemplating adding CommonJS Modules implementation to Rhino > >>> codebase proper. I'd create org.mozilla.javascript.commonjs package to > >>> hold > >>> it, and we could have a method similar to initStandardObjects(), i.e. > >>> initCommonJs() that'd initialize it - basically install a require() > >>> function > >>> with the expected semantics in the top-level scope. I want leave some of > >>> its > >>> aspects - most notably lookup of the module script - pluggable, defined > >>> by > >>> interfaces in the org.mozilla.javascript.commonjs package, so that > >>> specific > >>> embeddings of Rhino (JS app servers) can install their own module resolver > >>> logic. I'd provide a default implementation for the shell too. > > >>>> As I foresee that several Rhino-based JS products will adopt CommonJS in > >>> the near future, it seems desirable to not have all of them reinvent the > >>> wheel (even though some already did, I'm guilty of coding my own require() > >>> too in the next-gen version of my company's server-side JS enviroment...). > > >>>> Opinions? > > >>>> Attila. _______________________________________________ dev-tech-js-engine-rhino mailing list [email protected] https://lists.mozilla.org/listinfo/dev-tech-js-engine-rhino
