Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question about the content of module and module.children #27

Open
arisone opened this issue Oct 23, 2017 · 4 comments
Open

Question about the content of module and module.children #27

arisone opened this issue Oct 23, 2017 · 4 comments

Comments

@arisone
Copy link
Contributor

arisone commented Oct 23, 2017

I have the following script:

var module1= require('./module1);
var instance1= new module1.myclass1();
...
var module2= require('./module2);
var instance2= new module2.myclass2();

I pass the script to the engine (eval) and then I map each instance to a java class with the method getInterface(object, class).

The two modules and the two instances are created in the engineScope.
In the Module.class there is this part:

  private Module compileJavaScriptModule(Folder parent, String fullPath, String code)
      throws ScriptException {
    Bindings engineScope = engine.getBindings(ScriptContext.ENGINE_SCOPE);
    Bindings module = createSafeBindings();
    module.putAll(engineScope);

and then this other part:

  public Object require(String module) throws ScriptException {
    ...
    try {
      // If not cached, we try to resolve the module from the current folder, ignoring node_modules
      if (isPrefixedModuleName(module)) {
        found = attemptToLoadFromThisFolder(resolvedFolder, filename);  // this calls the method compileJavaScriptModule
      }
	...
    children.add(found.module);

So at the end I have module1 and instance1 among the children of module2. Obviously I have also all other things present in the engineScope (e.g. require, exports, and module <main>). The more I load module the more the structure becomes redundant.

Considering that I am new to Nashorn and node.js and there is no many information about module.children I'm questioning if it is correct to put the full content of the engineScope as child of a module.

Thanks for your attention.
aris

P.S. I've tried to comment module.putAll(engineScope): it works even so and the content of children seems more correct to me because I see only the module objects required by the one I'm inspecting.

@arisone arisone changed the title Question about content of module and so of module.children Question about content of module and module.children Oct 24, 2017
@arisone arisone changed the title Question about content of module and module.children Question about the content of module and module.children Oct 24, 2017
@arisone
Copy link
Contributor Author

arisone commented Oct 24, 2017

I've done a little test to explain well what I mean. I've created 4 modules (written in typescript and transpiled with module set to commonjs). The relation between modules is as follow:

  • module1 use subsubmodule
  • module2 use submodule which use subsubmodule

The test:

  • loads in this order module2 and module1
  • creates an instance of the class contained in each module
  • invokes a method on each instance
  • finally stringify the engineScope as Json

The test has been run both with the original code of nashorn-commonjs-modules and with the module.putAll(engineScope) commented.

The attached zip contains the java main to run the test, the four js modules, the two json produced from the runs.

modules-test.zip

@arisone
Copy link
Contributor Author

arisone commented Nov 3, 2017

I've found the moment when the change has been made:
Use real JS object whenever we need an instance of Bindings.
but I don't understand why the module.putAll(engineScope) has been added.
Thanks for your attention.

@malaporte
Copy link
Owner

Sorry, I haven't been much responsive, as mentioned in another issue I'm deep into house renovation work 😁

From what I remember, I added this part to copy the content of the global scope, because NashornScriptEngine.createBindings does some special magic when not running with the --global-per-engine flag (the default). Aka it allocates a brand new Global. But I've just tested with the line commented out in both modes, and all the UTs are passing, so I'm a bit confused. Certainly I haven't done this for no reasons...

I'll put those changes in a PR and do some more testing.

@arisone
Copy link
Contributor Author

arisone commented Nov 14, 2017

Don't worry @malaporte and thanks for your response. I'd like to contribute.

I've read this (Nashornjsr223 enginenotes --global-per-engineoption) and some other posts on the mailing list of Nashorn.

I've examined the diff between the current and the previous version of Module.
In the previous version, the call engine.createBindings was used to retrieve main objects (JSON, Object, Error) but in the current version they have been substituted by Module.createSafeBindings which internally calls objectConstructor.newObject().

The reference to objectConstructor and the others to main objects (engine included) are propagated from parent module to its children so the objects used are always the same.
Furthermore engine.createBindings is no longer invoked, avoiding any special treatments which depend on engine settings. This should explain why the UTs are passing.
For my current understanding it's similar to have the global-per-engine always set (in this way also the name of the method createSafeBindings make more sense to me).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants