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

Import module from evaluated script, ambiguous import and return value #1219

Closed
kpko opened this issue Jul 4, 2022 · 11 comments
Closed

Import module from evaluated script, ambiguous import and return value #1219

kpko opened this issue Jul 4, 2022 · 11 comments

Comments

@kpko
Copy link

kpko commented Jul 4, 2022

Hi,

I'm trying to use the new module feature in Jint but I can't get it to work as in other JavaScript engines (i.e. ClearScript). I want to evaluate a script thats returning a result and import methods from a c# based module.

using System.Reflection;
using Jint;

var engine = new Engine();

var module = new TestModule();
engine.AddModule("test", b =>
{
    b.ExportObject("module", module);
    foreach (var method in module.GetType().GetMethods(BindingFlags.Public | BindingFlags.Instance))
    {
        b.AddSource($"export function {method.Name}() {{ module.{method.Name}.apply(module, arguments) }}");
    }
});

var result = engine.Evaluate("import {TestMethod} from 'test'; return TestMethod()");
Console.WriteLine(result);

public class TestModule
{
    public void TestMethod()
    {
        Console.WriteLine("Hello, World!");
    }
}

When I try to evaluate the code snippet, I get an exception that modules can't be imported outside of a module. Can I somehow indicate, that the script that I want to evaluate should be evaluated in a "module context"? Also, is there a better way to expose every method/member of an object except for the workaround I implemented with AddSource?

I tried creating the snippet as a module itself and evaluate it.

engine.AddModule("main", b => b.AddSource("import {TestMethod} from 'test'; TestMethod()"));
var i = engine.ImportModule("main");

Do I have access to the completion value of the script, i.e. the result of testMethod in my case? Can I return from a module or do I have to use export default?

Thanks in advance. I'm excited to see if my use case works in Jint as it does in ClearScript :)

@lahma
Copy link
Collaborator

lahma commented Jul 4, 2022

Pinging @christianrondeau for possible insights.

@christianrondeau
Copy link
Contributor

Returning a value in a module isn't supported in node.js. For example, when you execute a script return 0; with node script.mjs it will throw SyntaxError: Illegal return statement.

I would recommend using export default TestMethod(); or export const result = TestMethod(); instead of relying on the completion value.

This being said, it wouldn't be (probably) very difficult to give access to the return value, but it would be tricky to provide a clean way to access it. Right now you can see in CyclicModuleRecord.Evaluate, the result of InnerModuleEvaluation is the completion you're looking for, but unless there's an error it's dropped in favor of the module loading promise. It'd be possible to make a method like "EvaluateModule" that would return the completion and drop the promise (weird) or "EvaluateAndImportModule" which would return both, if the maintainers agree; but it would not be standard.

Or, implement the ability to import a module in a script, which is also unsupported in the ecmascript specifications but would probably be desirable (but not necessarily obvious in the code, especially for async modules). That's probably what ClearScript does. I think @lahma and @sebastienros wanted that too (being able to import a module from a script), so I'm sure a PR would be welcome :) If you (or someone) wants to try, ping me I can share what my problems were in doing this (though it's been a while).

@lahma
Copy link
Collaborator

lahma commented Jul 4, 2022

Thanks @christianrondeau for the prompt and detailed response! I was hoping that Mr Modules had the answers ready in his sleeve 😉

@kpko
Copy link
Author

kpko commented Jul 4, 2022

Hi,

thank you very much for the quick answer. I tried implementing my use case with export default and that technically works. My remaining problem would be, that I have many, short-lived code snippets with imports that I want to use.

I could either call AddModule("main"...) multiple times with different code snippets, that would not work because the module only gets imported once (which seems like the way it should work), or I could call AddModule("script1...), AddModule("script2"...) and so on, which would pollute the engine a lot.

The ability to import a module in a script would indeed solve these problems. When I did this in ClearScript I couldn't use "return x" in my code but I needed to use just "x" as the last expression in my script to be able to retrieve the value.

Is there an existing branch with your experiment where you ran into problems that I could look at? I would like to tinker with the functionality but I'm not sure I can provide a pull request yet.

ClearScript has this notion of DocumentInfos that I can attach to scripts. When I declare my script as a module, I can just execute my script like I'm used to and the last expression gets returned.

var docInfo = new DocumentInfo() { Category = ModuleCategory.Standard };
var result = _engine.Evaluate(docInfo, "import {testMethod} from 'test'; testMethod()");
// result == result of the testMethod() call, because it's the last expression in the script

Thanks again :)

@christianrondeau
Copy link
Contributor

christianrondeau commented Jul 4, 2022

Your goal is to reuse the "C# modules"? My own use case is similar, however I create an Engine instance for each script, which also avoids scripts polluting other scripts (since modules are "live", i.e. a script could overwrite a module's object values). Also, if you can call scripts in a multithreaded environment (e.g. a web server) you cannot reuse the Engine anyway.

If your code is synchronous, another option would be to have a way to delete module cache entries, though I don't know if that would be enough to actually clear the entries from memory.

My own personal opinion (I'm not a maintainer by the way, so take everything I say with a grain of salt!) would be to first try validating if using an Engine instance per script would actually make a difference in terms of performance. If it does, then being able to "cleanup" modules from memory would allow some kind of "temporary modules" without too much effort, something like engine.ExecuteModule(builder => builder.AddSource("")) which would simply add, import, and delete the module from the cache. Just keep in mind that the result object actually attaches everything in that module, and the GC will not be able to clean that up until you also release this object. This would need to be tested with a WeakReference and GC.Collet to make sure it works in a unit test.

I have no branch for the issues I had, it's not that I implemented it in scripts and I had problems, it's more about not being able to actually implement it :) Long story short, modules behave very differently from scripts. Modules will be linked recursively, then evaluated, and the import statement will take care of linking the constant values from the current module scope. If you import a module into a script, then you'd actually create const variables, and this brought a lot of small changes here and there for something that was not defined in the standard in the first place (see #1102 for more details). But it can be done, if someone wants to write the small hacks here and there :)

Something you could also check is the require function, which can import module. I think it works too but it doesn't support async modules as far as I remember.

@lahma
Copy link
Collaborator

lahma commented Jul 21, 2022

Had a faint memory recently, was this one of the cases that one should use the dynamic import (const m = import('./my-module.js')) ? 🤔

@christianrondeau
Copy link
Contributor

You're right, I forgot we did support dynamic import: https://github.com/sebastienros/jint/blob/main/Jint.Tests/Runtime/ModuleTests.cs#L94

@lahma
Copy link
Collaborator

lahma commented Jul 25, 2022

@kpko have you had the chance to try out the import() call?

@kpko
Copy link
Author

kpko commented Jul 30, 2022

Yes, I'm using import() in my scripts now and it seems to work great. I lose some interoperability because the syntax is different from V8 and we can't use the same import {} from.. statements. It would be great if we would have some support for that in the future :)

Thank you.

@lahma
Copy link
Collaborator

lahma commented Jul 30, 2022

The import syntax should be standard, so sounds bad if it doesn't work the same way between the runtimes.

@lahma
Copy link
Collaborator

lahma commented Oct 20, 2022

Closing this as now import syntax is available and some import fixes have been done. Please open more specific issues if needed.

@lahma lahma closed this as completed Oct 20, 2022
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

3 participants