Yeah, returning null in ScriptProviders and ScriptSourceProviders allows them 
to be composed and fall back on each other without exceptions being thrown and 
caught. It's only when it'd surface as nonexistent from require() that the 
error is thrown.

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,
>> 
>> 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.
_______________________________________________
dev-tech-js-engine-rhino mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-js-engine-rhino

Reply via email to