CommonJS Modules in Rhino
Attila Szegedi
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.
Rapha
http://wiki.commonjs.org/wiki/Modules/1.1 ) ?
Raphael
Jarosław Pałka
Jarek
2010/1/18 Rapha <rsp...@gmail.com>
> _______________________________________________
> dev-tech-js-engine-rhino mailing list
> dev-tech-js-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-tech-js-engine-rhino
>
--
Bloguje się tutaj http://primitive.jogger.pl
A czatuje tutaj jpa...@jabber.gda.pl
Mark Porter
> I'm contemplating adding CommonJS Modules implementation to Rhino codebase proper.
> ...
> Attila.
+1
Sounds fantastic. Instant basic CommonJS support for all Rhino users.
I have been using Narwhal, which is fantastic, but this would be a
much more lightweight solution to the basic "require" needed to
bootstrap CommonJS.
----------------------------------------------------------
Mark Porter
Myna JavaScript Application Server
Easy server-side JS on a Java platform
http://www.mynajs.org
Martin Blom
> 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?
>
As long as the code embedding Rhino is responsible for resolving the
module files (using it's own search paths, for instance): Sounds great!
--
---- Martin Blom --------------------------- mar...@blom.org ----
Eccl 1:18 http://martin.blom.org/
Michael Mathews
JsDoc Toolkit.
----
Michael Mathews
Attila Szegedi
public interface ModuleScriptProvider
{
public Script getModuleScript(Context cx, String scriptId);
}
and the embedding code would be responsible for providing an implementation of it.
Attila.
Nathan
This sounds great!
Attila Szegedi
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 <rsp...@gmail.com>
>
Attila Szegedi
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.
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 <rsp...@gmail.com>
>>
George Moschovitis
Great idea!
It would be wise to consult with Kris Kowal on the actual
implementation.
-g.
Hannes Wallnoefer
> 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 <rspe...@gmail.com>
>
> >>> I like the idea. Are you thinking of the 1.1 spec (
> >>>http://wiki.commonjs.org/wiki/Modules/1.1) ?
Attila Szegedi
> On Jan 31, 8:15 pm, Attila Szegedi <szege...@gmail.com> 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.
First of all, thank you very much for taking the time to review it.
> The general code layout seems clear and well thought out.
Thanks; I have the habit of obsessing over details before I release something; I significantly transformed the damn thing several times before I was satisfied that I dare show it to other people :-)
> 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.
Well, I'm open to that if better suggestions come up for my current terms "module source" and "module script" - the former represents source code, the latter represents a compiled object (actually holds an instance of org.mozilla.javascript.Script). I attribute high significance to the identifiers I use - code legibility is a very important aspect for me; I did rename many of those classes several times before I dared to put the patch out :-). I.e. "ModuleScript" used to be just "Module", but then I realized that in this terminology, "module" is a JS object, an instance particular to an execution of the program, hence the class I had there couldn't be named "Module"... you can see how it goes.
So anyway, what I wanted to say is, I'm open to better naming suggestions. I don't like "resource" probably because I'm indoctrinated with REST beyond hope of recovery - any particular source text retrieved through an URL is at best "a text entity that is the representation of the resource at the time of retrieval". See, I told you I'm beyond hope. I was considering "ModuleSourceEntity", but then settled for "ModuleSource" :-)
> 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.
Well, there's File.lastModified(); we have to settle for it... As a matter of fact, I'm still thinking that I would need to force non-caching of any resource where last-modified is less than two seconds before the retrieval, to make sure we don't miss multiple updates within a single second with an underlying resource provider that only provides one-second resolution (most filesystems are better, but HTTP doesn't go below one-second resolution, and indeed HTTP/1.1 spec explicitly recommends this practice for caching).
> At least I think there should be a
> FileModuleSourceProvider in addition to the UrlModuleSourceProvider.
Possibly - my thinking was that using the UrlModuleSourceProvider with file: URLs yields the same results as a dedicated java.io.File-based provider would, and there's less code to maintain (I *love* cutting down on code; "functionality is asset, code is liability" is my mantra). Using files through file: URLs is not noticeably less efficient than using java.io.File API directly. Granted, it'll set an If-Modified-Since header in vain on revalidation checks, and there'll be one gratuitous open/close of the file involved, but that's pretty much the only additional overhead (and this only happens once per expiry duration, cca, every 60 seconds by default). Of course, a "FileModuleSourceProvider extends ModuleSourceProviderBase" would indeed be easy to implement.
> 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.
:-)
Is there a way to reconcile the differences? I tried to follow Modules/1.1 to the letter... Also, maybe your implementation is doing something better than mine does, in which case it wouldn't be bad if I'd get some inspiration the other way :-)
Attila.
Chris Zumbrunn
> On 2010.02.03., at 17:52, Hannes Wallnoefer wrote:
>
>> 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.
>
ModuleSource and ModuleCompiled ?
Chris
Jarosław Pałka
I was playing with your patch for few days, and one thing I found is
annoying NullPointerException when module is not found :), I believe we
should have more meaningful exception message.
I am also working on tests, using http://code.google.com/p/interoperablejs/.
Were you trying to run your patch against these tests?
So far code looks good,
Regards,
Jarek
W dniu 31 stycznia 2010 20:15 użytkownik Attila Szegedi
<szeg...@gmail.com>napisał:
> 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.
>
> 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 <rsp...@gmail.com>
> _______________________________________________
> dev-tech-js-engine-rhino mailing list
> dev-tech-js-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-tech-js-engine-rhino
>
--
Attila Szegedi
> Attila,
>
> I was playing with your patch for few days, and one thing I found is annoying NullPointerException when module is not found :), I believe we should have more meaningful exception message.
>
> I am also working on tests, using http://code.google.com/p/interoperablejs/. Were you trying to run your patch against these tests?
Hi,
No, I haven't, but I definitely will now. I'm writing JUnit tests. I just recently discovered that there's an EMMA plugin for Eclipse, it greatly improves my progress towards good coverage. I have found few bugs myself.
Also, I have found a way to make Require thread-safe for use with shared top level scopes. And by "found a way" I mean "found a more efficient way than using a coarse synchronized block on it" :-)
Attila.
Jarosław Pałka
I have finished tests with interoperablejs. I wonder how should I share it
with you. Should I attach tests suite to bugzilla issue?
Jarek
W dniu 8 lutego 2010 08:14 użytkownik Attila Szegedi
<szeg...@gmail.com>napisał:
Attila Szegedi
yes, please. I did start working on a JUnit driver for them myself, but if you already have it completed, I'll be happy to accept your patch and save me some work.
Thanks,
Attila.
Jarosław Pałka
interoperablejs. I don't think that having dependencies to external
SVN repo is a good solution. I think the best way is to simply commit
content of http://interoperablejs.googlecode.com/svn/trunk into
mozilla/js/rhino/testsrc/interoperablejs. This way we can also do a
changes in tests itself.
Let me know what do you think about it.
Regards,
Jarek
W dniu 8 lutego 2010 08:59 użytkownik Attila Szegedi
> > _______________________________________________
> > dev-tech-js-engine-rhino mailing list
> > dev-tech-js-...@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-tech-js-engine-rhino
--
Attila Szegedi
<target name="get-interoperablejs">
<exec executable="svn" dir="build/test">
<arg value="checkout"/>
<arg value="http://interoperablejs.googlecode.com/svn/trunk/"/>
<arg value="interoperablejs-read-only"/>
</exec>
</target>
to testsrc/build.xml and making junit depend on it:
<target name="junit" depends="junit-compile,coverage-instrument,get-interoperablejs">
...
This makes it reside in build/test/interoperablejs-read-only. Alternatively, we could have it go under testsrc/org/mozilla/javascript/commonjs/module. But I'm really not a fan of hosting 3rd party code in Mozilla CVS, for legal reasons. Yes, I know InteroperableJS is MIT licensed, but still.
Attila.
Jarosław Pałka
I will include it in my patch,
Jarek
W dniu 8 lutego 2010 10:52 użytkownik Attila Szegedi
--
Attila Szegedi
Also, just this morning i put out a new patch at <https://bugzilla.mozilla.org/show_bug.cgi?id=540724> that already has all the tests I wrote in it - this likely makes your work unnecessary; I'm sorry I didn't wait on you, but I have scarce spare time to work on this, so when I do have, I take advantage of it... By all means though, give it a review. I did end up taking your approach and including the files, as the new set of files wasn't available through command line SVN...
Anyway, others might want to take the opportunity to check the current patch - it's now tested with 60% coverage; the untested stuff is mostly to do with cache revalidation. It passes all the compliance tests + my additional implementation tests.
Also, a big improvement compared to the previous patch is that I made require() threadsafe, with good concurrent use properties, while not sacrificing performance for single threaded use. So it can now be used to load modules on demand into a shared top-level scope (which is something many server-side JS embeddings do).
Attila.
--
home: http://www.szegedi.org
twitter: http://twitter.com/szegedi
weblog: http://constc.blogspot.com
Jarosław Pałka
I was busy last few days with my daily work, in this case I will check
your patch and put comments.
In the meantime, I thought about integrating modules with
ScriptableObject. What I mean by that? Currently in patch we have a
nice way to load modules from JavaScript files.I think it will be nice
to have a way to define modules as set of implementations of
Scriptable interface.
I thought about following solution. We will have package level
annotation, called @Exports (to be aligned with Modules naming
convention). This annotation will point to classes which instances
will be exported by module. Let code speaks :)
in package-info.java file:
@Exports(MyFirstObject.class,MySecondObject.class)
package com.myproject.mypackage;
and in JavaScript:
var test = require("com/myproject/mypackage");
var obj = new test.MyFirstObject();
What do you think about it?
Jarek
W dniu 10 lutego 2010 12:43 użytkownik Attila Szegedi
Jarosław Pałka
I applied third version of your patch. I have two comments:
- I don't see where you execute interoperablejs tests, I see them in
source code but it looks like they are not run as part of junit-all
target Maybe I missed something :(
- In CachingModuleScriptProviderBase, in line 70, you return null when
module source is null, according to CommonJS spec we should throw error
when module was not found
It looks great after your last refactoring :)
Regards,
Jarek
Attila Szegedi pisze:
> Actually, I was informed on the commonjs list that there's a new set of compliance tests at <http://github.com/kriskowal/commonjs/tree/master/tests/modules/1.0/> and the one on interoperablejs is obsolete.
>
> Also, just this morning i put out a new patch at <https://bugzilla.mozilla.org/show_bug.cgi?id=540724> that already has all the tests I wrote in it - this likely makes your work unnecessary; I'm sorry I didn't wait on you, but I have scarce spare time to work on this, so when I do have, I take advantage of it... By all means though, give it a review. I did end up taking your approach and including the files, as the new set of files wasn't available through command line SVN...
>
> Anyway, others might want to take the opportunity to check the current patch - it's now tested with 60% coverage; the untested stuff is mostly to do with cache revalidation. It passes all the compliance tests + my additional implementation tests.
>
> Also, a big improvement compared to the previous patch is that I made require() threadsafe, with good concurrent use properties, while not sacrificing performance for single threaded use. So it can now be used to load modules on demand into a shared top-level scope (which is something many server-side JS embeddings do).
>
> Attila.
>
> --
> home: http://www.szegedi.org
> twitter: http://twitter.com/szegedi
> weblog: http://constc.blogspot.com
>
> On 2010.02.10., at 12:31, Jaros�aw Pa�ka wrote:
>
Jarosław Pałka
Jaros�aw Pa�ka pisze:
Martin Blom
> 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 and list,
I've started playing with the patch in the context of my ESXX project
and while I haven't had time to finish yet I have a couple of questions:
1) I already have a path array in ESXX, which also happens to be a JS
array that is searched from the front. However, having two path objects
is weired, so I'm wondering if we could either perhaps replace the
'secure' argument with a 'paths' argument (Scriptable) so I can inject
mine, or add a method that returns Require's paths object so I can use
that one instead?
2) I have a special property named 'esxx.location' that contains the URI
of the currently executing file. For this to work I need to know when
the Script is executed. It could be solved by making
Require.executeModuleScript protected, or better, adding an exec method
to ModuleScript.
Comments?
Martin Blom
> 1) I already have a path array in ESXX, which also happens to be a JS
> array that is searched from the front. However, having two path objects
> is weired, so I'm wondering if we could either perhaps replace the
> 'secure' argument with a 'paths' argument (Scriptable) so I can inject
> mine, or add a method that returns Require's paths object so I can use
> that one instead?
(I just realized that my array is not an array of string but URI
objects, so I think I need to provide my own Scriptable that implements
the correct view of the path array.)
Attila Szegedi
Attila.
On 2010.02.10., at 20:12, Jarosław Pałka wrote:
> Ok, you throw exception in Require :), in getModule() :) nice trick
>
> Jarosław Pałka pisze:
Attila Szegedi
On 2010.02.10., at 19:38, Jarosław Pałka wrote:
> Attila,
>
> I applied third version of your patch. I have two comments:
> - I don't see where you execute interoperablejs tests, I see them in source code but it looks like they are not run as part of junit-all target Maybe I missed something :(
No, I think it's me who missed something - in testsrc/build.xml, just under:
<copy todir="${coverage.classes.dir}">
<fileset dir="src" excludes="**/*.java"/>
</copy>
we need to add this too:
<copy todir="${coverage.classes.dir}">
<fileset dir="testsrc" excludes="**/*.java"/>
</copy>
I have to admit that I run my unit tests from Eclipse IDE, so didn't run into this problem...
Attila.
> - In CachingModuleScriptProviderBase, in line 70, you return null when module source is null, according to CommonJS spec we should throw error when module was not found
>
> It looks great after your last refactoring :)
>
> Regards,
> Jarek
>
> Attila Szegedi pisze:
>> Actually, I was informed on the commonjs list that there's a new set of compliance tests at <http://github.com/kriskowal/commonjs/tree/master/tests/modules/1.0/> and the one on interoperablejs is obsolete.
>>
>> Also, just this morning i put out a new patch at <https://bugzilla.mozilla.org/show_bug.cgi?id=540724> that already has all the tests I wrote in it - this likely makes your work unnecessary; I'm sorry I didn't wait on you, but I have scarce spare time to work on this, so when I do have, I take advantage of it... By all means though, give it a review. I did end up taking your approach and including the files, as the new set of files wasn't available through command line SVN...
>>
>> Anyway, others might want to take the opportunity to check the current patch - it's now tested with 60% coverage; the untested stuff is mostly to do with cache revalidation. It passes all the compliance tests + my additional implementation tests.
>>
>> Also, a big improvement compared to the previous patch is that I made require() threadsafe, with good concurrent use properties, while not sacrificing performance for single threaded use. So it can now be used to load modules on demand into a shared top-level scope (which is something many server-side JS embeddings do).
>>
>> Attila.
>>
>> --
>> home: http://www.szegedi.org
>> twitter: http://twitter.com/szegedi
>> weblog: http://constc.blogspot.com
>>
>> On 2010.02.10., at 12:31, Jarosław Pałka wrote:
>>
>
Attila Szegedi
> On 31/01/2010 20:15, Attila Szegedi 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 and list,
>
> I've started playing with the patch in the context of my ESXX project and while I haven't had time to finish yet I have a couple of questions:
>
> 1) I already have a path array in ESXX, which also happens to be a JS array that is searched from the front. However, having two path objects is weired, so I'm wondering if we could either perhaps replace the 'secure' argument with a 'paths' argument (Scriptable) so I can inject mine, or add a method that returns Require's paths object so I can use that one instead?
You can easily obtain the paths of a require instance. Remember, a require() function is just a JS object, so you can do:
Require require = ...;
NativeArray paths = ScriptableObject.getTypedProperty(require, "paths", NativeArray.class);
> 2) I have a special property named 'esxx.location' that contains the URI of the currently executing file. For this to work I need to know when the Script is executed. It could be solved by making Require.executeModuleScript protected, or better, adding an exec method to ModuleScript.
I see - you need a post-exec hook. Okay, that's a reasonable thing. I actually at one point in time had ModuleScript.exec(), but I scrapped it. The reason is in the way SoftCachingModuleScriptProvider is implemented; if you look into it, ModuleScript instances in it are not even preserved, but they are deconstructed/reconstructed into/from the ScriptReference as needed. This is done so that there's no strong reference Script objects being held, so unused scripts can indeed get unloaded. (functions have pointers to their parent scripts, so as long as there's any function from the script reachable in any of the currently running JS program instance, they won't get GCed, otherwise they can be). For the same reason, you shouldn't create such post-exec hook by creating a wrapper around Script that itself implements Script with pre/post processing - you'd screw up soft caching as you can't sneak in a strong reference to your wrapper Script object into the Script object that Rhino compiles for you, which'd be required to ensure soft cache doesn't load multiple reachable copies of scripts (& functions).
So, an explicit way to register a post-exec hook is the cleanest solution - I'll look into where would this fit best.
Thank you very much for taking time to look into this and make suggestions.
Attila.
--
twitter: http://twitter.com/szegedi
weblog: http://constc.blogspot.com
home: http://www.szegedi.org
> Comments?
Attila Szegedi
Given that the current implementation state seems sufficiently stable, I committed it to CVS HEAD and will continue development there.
The committed version features following changes compared to last patch:
- Require constructor can now accept a pre-exec and a post-exec Script; these are executed before and after any module script, respectively. This is in response to Martin Blom's requirement for a post-exec hook. Martin, can you confirm this is something that suits your needs?
- RequireBuilder has been moved to the base package, seeing how Require constructor now takes quite a lot of arguments (which, for all apparent unwieldiness, is how I like it, as I love immutable objects when possible). I expect people will prefer to use RequireBuilder instead of invoking the Require constructor directly, although of course, you can still do that just as well.
- Renamed the org.mozilla.javascript.commonjs.module.support package to *.provider, as all the classes in it actually deal with module provider implementation (now that RequireBuilder has been moved out of it).
- added a MultiModuleScriptProvider, for easy composition of multiple module source providers into a single one. This is meant to open the door for modular implementation of advanced module provision methods, above and beyond loading them from .js source files. I.e. Jarosław's idea of annotations-based Java-defined modules can be implemented as a separate provider, and composed with a more conventional provider into a single composite using the MultiModuleScriptProvider. Jarosław, care to provide the appropriate ModuleScriptProvider implementation?
Next on the roadmap is using the stuff available so far to add require() to Rhino shell. For compatibility, it'll probably require a new command-line switch and be off by default. I'll outline the further ideas soon.
Attila.
--
twitter: http://twitter.com/szegedi
weblog: http://constc.blogspot.com
home: http://www.szegedi.org
On 2010.02.10., at 12:43, Attila Szegedi wrote:
> Actually, I was informed on the commonjs list that there's a new set of compliance tests at <http://github.com/kriskowal/commonjs/tree/master/tests/modules/1.0/> and the one on interoperablejs is obsolete.
>
> Also, just this morning i put out a new patch at <https://bugzilla.mozilla.org/show_bug.cgi?id=540724> that already has all the tests I wrote in it - this likely makes your work unnecessary; I'm sorry I didn't wait on you, but I have scarce spare time to work on this, so when I do have, I take advantage of it... By all means though, give it a review. I did end up taking your approach and including the files, as the new set of files wasn't available through command line SVN...
>
> Anyway, others might want to take the opportunity to check the current patch - it's now tested with 60% coverage; the untested stuff is mostly to do with cache revalidation. It passes all the compliance tests + my additional implementation tests.
>
> Also, a big improvement compared to the previous patch is that I made require() threadsafe, with good concurrent use properties, while not sacrificing performance for single threaded use. So it can now be used to load modules on demand into a shared top-level scope (which is something many server-side JS embeddings do).
>
> Attila.
>
> --
> home: http://www.szegedi.org
> twitter: http://twitter.com/szegedi
> weblog: http://constc.blogspot.com
>
> On 2010.02.10., at 12:31, Jarosław Pałka wrote:
>
>>>>> On 2010.02.08., at 8:57, Jarosław Pałka wrote:
>>>>>
>>>>>> Attila,
>>>>>>
>>>>>> I have finished tests with interoperablejs. I wonder how should I share it with you. Should I attach tests suite to bugzilla issue?
>>>>>>
>>>>>> Jarek
>>>>>>
>>>>>> W dniu 8 lutego 2010 08:14 użytkownik Attila Szegedi <szeg...@gmail.com> napisał:
>>>>>> On 2010.02.07., at 20:33, Jarosław Pałka wrote:
>>>>>>
>>>>>>> Attila,
>>>>>>>
>>>>>>> I was playing with your patch for few days, and one thing I found is annoying NullPointerException when module is not found :), I believe we should have more meaningful exception message.
>>>>>>>
>>>>>>> I am also working on tests, using http://code.google.com/p/interoperablejs/. Were you trying to run your patch against these tests?
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> No, I haven't, but I definitely will now. I'm writing JUnit tests. I just recently discovered that there's an EMMA plugin for Eclipse, it greatly improves my progress towards good coverage. I have found few bugs myself.
>>>>>>
>>>>>> Also, I have found a way to make Require thread-safe for use with shared top level scopes. And by "found a way" I mean "found a more efficient way than using a coarse synchronized block on it" :-)
>>>>>>
>>>>>> Attila.
>>>>>>
>>>>>> --
>>>>>> home: http://www.szegedi.org
>>>>>> twitter: http://twitter.com/szegedi
>>>>>> weblog: http://constc.blogspot.com
>>>>>>
>>>>>>>
>>>>>>> So far code looks good,
>>>>>>> Regards,
>>>>>>> Jarek
>>>>>>>
>>>>>>> W dniu 31 stycznia 2010 20:15 użytkownik Attila Szegedi <szeg...@gmail.com> napisał:
>>>>>>> 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.
>>>>>>>
>>>>>>> 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
>>>>>>>>
Hannes Wallnoefer
On Feb 3, 6:39 pm, Attila Szegedi <szege...@gmail.com> wrote:
> On 2010.02.03., at 17:52, Hannes Wallnoefer wrote:
>
> > 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.
>
> :-)
>
> Is there a way to reconcile the differences? I tried to follow Modules/1.1 to the letter... Also, maybe your implementation is doing something better than mine does, in which case it wouldn't be bad if I'd get some inspiration the other way :-)
>
I put a lot of work and thought into the design of Helma NG (which
we're currently relaunching as RingoJS). As a result, it is a bit
farther away from the "classical" JS shell, but there are some very
tangible benefits.
For example, module scopes are top level scopes in HNG/RingoJS. This
gives as complete module isolation even with a shared "classical"
global scope. Also, the require() function in Helma NG is shared
among multiple contexts and threads, and we're using a threadlocal to
keep maps of loaded modules per cx/thread.
This gives us a lot of flexibility. For example, while modules are
instantiated per context/thread by default, modules can opt to have
one instance shared among all threads simply by setting a flag in
their module meta-object, giving us optionally multithreaded modules.
There's also completely transparent reloading of modules that takes
care of module dependencies for shared modules.
To sum it up, HNG/RingoJS aims a bit farther away from the typical JS
shell/runtime. That's why I think it should remain a dedicated project
(I've also thought about proposing our module loader for inclusion in
Rhino at certain times). But I also think your implementation makes
perfect sense to be included in Rhino for more "conservative" CommonJS
runtimes.
Hannes
Martin Blom
Hi again,
Sorry for taking so long.
I've started playing with the patch in the context of my ESXX project and while I haven't had time to finish yet I have a couple of questions: 1) I already have a path array in ESXX, which also happens to be a JS array that is searched from the front. However, having two path objects is weired, so I'm wondering if we could either perhaps replace the 'secure' argument with a 'paths' argument (Scriptable) so I can inject mine, or add a method that returns Require's paths object so I can use that one instead?You can easily obtain the paths of a require instance. Remember, a require() function is just a JS object, so you can do: Require require = ...; NativeArray paths = ScriptableObject.getTypedProperty(require, "paths", NativeArray.class);
Anyway, I ended up simply require.delete("paths") for now. From what I see, the paths property is optional according to CommonJS.
2) I have a special property named 'esxx.location' that contains the URI of the currently executing file. For this to work I need to know when the Script is executed. It could be solved by making Require.executeModuleScript protected, or better, adding an exec method to ModuleScript.I see - you need a post-exec hook. Okay, that's a reasonable thing. I actually at one point in time had ModuleScript.exec(), but I scrapped it. The reason is in the way SoftCachingModuleScriptProvider is implemented; if you look into it, ModuleScript instances in it are not even preserved, but they are deconstructed/reconstructed into/from the ScriptReference as needed. This is done so that there's no strong reference Script objects being held, so unused scripts can indeed get unloaded. (functions have pointers to their parent scripts, so as long as there's any function from the script reachable in any of the currently running JS program instance, they won't get GCed, otherwise they can be). For the same reason, you shouldn't create such post-exec hook by creating a wrapper around Script that itself implements Script with pre/post processing - you'd screw up soft caching as you can't sneak in a strong reference to your wrapper Script object into the Script object that Rhino c ompiles for you, which'd be required to ensure soft cache doesn't load multiple reachable copies of scripts (& functions). So, an explicit way to register a post-exec hook is the cleanest solution - I'll look into where would this fit best.
- The code I'm interested in running is Java, not JS
- No reference to the ModuleScript in question is passed to the pre/post hooks
- I would need hold state using a custom stack in order to restore
the location in the post-hook
public class ESXXScript
extends ModuleScript
implements Script {
public ESXXScript(Script script, URI uri) {
super(script, uri.toString());
this.uri = uri;
}
@Override public Script getScript() {
return this;
}
@Override public Object exec(Context cx, Scriptable scope) {
URI old_location = setCurrentLocation(uri);
try {
return super.getScript().exec(cx, scope);
}
finally {
setCurrentLocation(old_location);
}
}
private URI uri;
}
However, I still think it would be better if ModuleScript
extended Script and used public Object exec(Context cx,
Scriptable scope) instead of public Script getScript().
I can't see how that would prevent soft-references from working as
intended, but anyway, ESXXScript works for me and I'm happy
with it as it is.More importantly, however, is the installation of the main module. I'm setting up the application scope using custom code and not loading it as a module, so Require.requireMain() is not applicable here. Require.install() only installs the require() function, it does not set up a main module.
I modified Require() to include an installMain() method. Patch attached. Would you consider to include it? (The patch is against your 3rd Bugzilla patch; I'm on the Rhino 7r2 branch.)