Hi, in nodejs and avatar-is such declarations are leaked. Actually 100% of node core scripts use var. Declaring variables without var seems something that has to be chased and eradicated more than to be supported. A global "global" is used to add global definitions. I would say that all node modules are following this rule. If they are not, this is a bug in the script.
JF On Dec 11, 2013, at 2:22 PM, Tim Fox <[email protected]> wrote: > On 11/12/13 13:19, A. Sundararajan wrote: >> The way Avatar/js project ( https://java.net/projects/avatar-js ) project >> implements CommonJS/require is as follows. >> >> It creates a anonymous function code wrapping a module (say like http.js). >> The anonymous function accepts 'exports' as argument. When you eval that >> code at top level global scope, because of the anon function wrapping >> around, all top level vars in a module code like http.js become locals of >> that anon function. > > Isn't that the same as what I described in my last post? > > If so, the problem with that is that globals that aren't prefixed with var > still leak. > >> Only whatever is passed as 'exports' is populated with exposed functions and >> variables. The caller can pass suitable 'exports' object as argument when >> calling the anon functions. >> >> -Sundar >> >> On Wednesday 11 December 2013 06:34 PM, Tim Fox wrote: >>> On 11/12/13 12:53, Attila Szegedi wrote: >>>> On Dec 11, 2013, at 1:13 PM, Tim Fox <[email protected]> wrote: >>>> >>>>> Confused... >>>>> >>>>> I assumed that if two scripts where run with their own script context, >>>>> then they would already have separate globals, i.e. if I do >>>>> >>>>> myglobal = 1 >>>>> >>>>> in module 1 that won't be visible in module 2. >>>> That's true, but then you also end up with the need for >>>> ScriptObjectMirrors between them, and that was what I suggested you try to >>>> avoid. >>>> >>>>> So I'm not sure really what --global-per-engine really means, if the >>>>> modules have their own globals anyway. I guess my understanding must be >>>>> wrong somewhere. >>>> Well, it will mean that modules won't have their own globals… >>>> --global-per-engine will make it so that the Global object is specific to >>>> the ScriptEngine, and not to the ScriptContext, e.g. even if you replace >>>> the ScriptContext of the engine, when scripts are run, they'll still see >>>> the same global object as before the replacement. The gist of it is: >>>> >>>> a) without --global-per-engine, the Global object is specific to a >>>> ScriptContext, each ScriptContext has its own. ENGINE_SCOPE Bindings >>>> object of the context is actually a mirror of its Global. >>>> b) with --global-per-engine, the Global object lives in the ScriptEngine. >>>> ENGINE_SCOPE Bindings object of the context is just a vanilla >>>> SimpleBindings (or whatever you set it to), and Global will delegate >>>> property getters for non-existent properties to it (but it'll still >>>> receive property setters, so new global variables created by one script >>>> will be visible by another; no isolation there). >>>> >>>> What I was suggesting is that your module loading code would look >>>> something like: >>>> >>>> // The engine that you use >>>> ScriptEngine engine = new NashornScriptEngine().getEngine(new String[] { >>>> "--global-per-engine" }); >>>> >>>> ... >>>> >>>> // when loading a module >>>> Bindings moduleVars = new SimpleBindings(); >>>> moduleVars.put("require", requireFn); >>>> moduleVars.put("module", moduleDescriptorObj); >>>> moduleVars.put("exports", exportsObj); >>>> Bindings prevBindings = engine.getBindings(ENGINE_SCOPE); >>>> engine.setBindings(moduleVars, ENGINE_SCOPE); >>>> try { >>>> engine.eval(moduleSource); >>>> } finally { >>>> engine.setBindings(prevBindings, ENGINE_SCOPE); >>>> } >>>> return exportsObj; >>>> >>>> NB: your modules would _not_ run in isolated globals. >>> >>> That's the problem. CommonJS modules need to run in their own globals to >>> avoid pollution. >>> >>>> I thought they do, but I just spoke to Sundar and he explained the >>>> mechanism to me so now I see they won't -- see above the case b). >>>> >>>> They could pollute each other's global namespace (since it's shared). >>>> Hopefully they'd adhere to Modules/Secure recommendation and refrain from >>>> doing so, but you can't really enforce it. >>> >>> Just about every CommonJS module I have seen uses globals, e.g. pretty much >>> all the node.js ones (and the vert.x ones). Here's an example: >>> >>> https://github.com/joyent/node/blob/master/lib/http.js >>> >>> Notice the liberal use of top level vars, which are globals. This is the >>> norm in CommonJS AIUI. Any require() implementation that doesn't provide >>> global isolation isn't going to fly. >>> >>> >>>> >>>> My require() implementation in Rhino could provide real isolation, but >>>> this is unfortunately impossible in Nashorn. Nashorn makes an assumption >>>> that the global object during execution of a script is an instance of >>>> jdk.nashorn.internals.objects.Global; in Rhino, it could have been >>>> anything so there I was able to run a module in a new scope that had the >>>> actual Global object as its prototype, so it could catch all variable >>>> assignments in itself and essentially make the Global read only (albeit >>>> objects in it mutable - e.g. a module could still extend Array prototype >>>> etc.). >>>> >>>> In Nashorn, it's the other way round with "--global-per-engine" - Global >>>> object is the one immediately visible to scripts, and ENGINE_SCOPE >>>> Bindings object is used as source of properties that aren't found in the >>>> Global. Here, Global catches variable assignments and ENGINE_SCOPE >>>> Bindings object ends up being immutable (although objects in it are >>>> obviously still mutable, so the module can build up its "exports" object). >>>> >>>> Basically, you have a choice between having shared globals (no isolation) >>>> without mirrors (and then you won't run into any issues with mirrors), or >>>> separate globals (with real isolation), but then also mirrors, and then >>>> you might run into limitations of mirrors (e.g. they can't be >>>> automatically used as Runnable etc. callbacks from Java and so on.) >>> >>> Mirrors also don't seem to work in the way required for a require() >>> implementation, as described in my previous experiments :( >>> >>> So... the way it looks right now, I can't see anyway I can create a working >>> commonJS require() implementation with Nashorn as it stands currently. This >>> would be a showstopper for having Nashorn support in Vert.x which is >>> dissappointing as I was impressed by the good performance :( >>> >>> Having said that, apparently avatar.js does implement require() so I'm >>> puzzled how it manages that. I did look at the avatar.js code but couldn't >>> find their require implementation. Does anyone have any pointers? >>>> >>>> I'm not trying to justify or otherwise qualify any of the design decisions >>>> here, just trying to help you understand its constraints. >>>> >>>> Attila. >>>> >>> >> >
