Support Esprima.Ast.ExportNamedDeclartion

[MeikelLP]

I want to write a Javascript file like

export var name = 'Mod1'

But this gives me:

InvalidCastException: Unable to cast object of type 'Esprima.Ast.ExportNamedDeclaration' to type 'Esprima.Ast.Statement'.
  at Jint.Runtime.Interpreter.JintStatementList.Initialize() in C:\projects\jint\Jint\Runtime\Interpreter\JintStatementList.cs:34

This is an ES6 feature.

[lahma]

Import/export should be covered in https://github.com/sebastienros/jint/issues/430 , I'll close this and we should continue tracking on the specific feature issue.

[christianrondeau]

It seems #430 was closed, but as far as I can tell it's still impossible to use something like engine.Execute("export const x = 12;");.

I guess it would also be useful to have a way to "get" those exported values. Right now the alternative is to do something like x = 12; which creates a global variable. Using const x = 12 makes x innaccessible from outside the JS code.

Should this be re-opened, did I miss something or should I open a new issue instead?

Thanks for the amazing project by the way :)

[lahma]

The module runtime behavior is still lacking, the static constructs and services are there, but runtime evaluation is missing some bits.

[christianrondeau]

I was considering investigating at least two things, so I'm not creating a monster :) My questions:

1. What do you think of the following? Would that be aligned with how Jint is supposed to work, or am I stretching?
2. do you have any pointers or ideas on how it should work, or should I start with a PR outlining the idea first?

Ability to use the export keyword and make exported objects visible in a dictionary on the engine.

Given the script

export const x = 5;

engine.Execute(script);

Assert.That(engine.Module.Exports["x"].AsInt(), Is.EqualTo(5));

(I'll work on the details such as default exports if that makes sense to you)

Ability to resolve C#-provided imports

import { MyType } from '@mycompany/whatever';

var engine = new Engine(opts => opts.AddModuleResolver<MyModuleResolver>());

public class MyModuleResolver : IModuleResolver {
    // This has to be fleshed out once I better understand how Esprima implements this
}

[lahma]

@christianrondeau I did small sprint one evening in the past checking what was missing, I've pushed the very WIP, a sketch here: https://github.com/sebastienros/jint/pull/1051 .

So what is needed to examine the spec and how things are supposed to work, in this module related runtime behavior like JintImportStatement which should follow https://tc39.es/ecma262/#sec-import-call-runtime-semantics-evaluation .

I've added a test case there too which starts to examine the runtime "integration test" kind of testing on top of current module handling logic.

There's already a IModuleLoader which you refer as IModuleResolver I believe. Things get a bit hairy when you start thinking in terms of node modules (CommonJS), but we should start with basic ES6 module support which is more straightforward.

[christianrondeau]

Thanks, that's helpful; in my case I didn't want to necessarily implement full-fledged modules handling as a first contribution, but from your PR I can already see the pieces that should be involved. I'll try and implement "export" first (it at least sounds simpler).

Quick questions though, so I can start on the right feet;

1. The file Language\ModuleTests.cs is already green, but it has Skip=true. Should I already make a PR to remove the skip attribute? Any reason the test suite is skipped?
2. ExtensionMethodsTests.LinqExtensionMethodWithSingleGenericParameter is not passing right now, is it just me?
3. Should this issue be re-opened until (if) I make an accepted PR?

Thanks :)

[lahma]

1. The file Language\ModuleTests.cs is already green, but it has Skip=true. Should I already make a PR to remove the skip attribute? Any reason the test suite is skipped?

[MemberData(nameof(SourceFiles), "language\\module-code", false)]
[MemberData(nameof(SourceFiles), "language\\module-code", true, Skip = "Skipped")]

Means that SourceFiles method will produce non-skipped (parameter false) and skipped (parameter true) test cases and will show them either skipped or ran in test runner output. There are some skipped tests related to features like import assertions that haven't (yet) been implemented.

2. ExtensionMethodsTests.LinqExtensionMethodWithSingleGenericParameter is not passing right now, is it just me?

Might be a problem with full framework.

3. Should this issue be re-opened until (if) I make an accepted PR?

I've reopened it. So the PR should probably produce a JinExportNamedDeclaration which implements the export runtime semantics.

Thanks for helping out!

[christianrondeau]

I'll need to bring in part of what you did, i.e. the concept of GetActiveScriptOrModule and friends. Are you OK with me bringing back whatever I need from that WIP? Or should I just build on top of it against your GitHub repo?

Technically I expect most of the code to be independent from yours, but I need to have access to the concept of "current module", which is what you've done already.

[lahma]

Take everything you need, there might be some errors etc, but whatever helps the effort.

[christianrondeau]

I think I opened a can of worms :D I can correctly parse the export syntax, but in the end, it doesn't really do anything ECMAScript-wise... so a few Jint-level decisions need to be made.

The "problem" is that Jint runs in a "script" even though there's no implementation for it (it's "taken for granted" from what I can see that the current context is a script, e.g. strict is optional, there's no underlying ScriptRecord or JsModule), so going by the book, we cannot support import/export :)

There's no import or export in a "script". Following ECMAScript semantics, I see a few options. I feel like option 3 is the most sensible and easier to use, and wouldn't break ECMAScript per se, but is still out of line with what I understand should be the behavior.

Option 1: Execute, GetValue runs on a script

This means, no import or export at all in Execute or GetValue, since this runs on a script. The idea would be to create a module instead if you want to use module features.

var module = engine.LoadModule("my-module", "export 'test';");
var value = module.GetDefaultExport().AsString();

To use both you have to use globals.

var module = engine.LoadModule("my-module", "globals.value = 'test';");
var value = module.Execute("globals.value").AsString();

Option 2: Make the "root" script a scriptOrModule

This would be more in line with ECMAScript but would require much heavier refactoring. Right now, Engine just runs statements one after the other, but there's no ScriptRecord or ModuleRecord. The idea would be to consider the "root" script like any other loaded file, i.e. it can either be a module or a script. That means while parsing the file, we can check for occurrences of import or export and change the context (and set strict to true). That would be nicer though since we can do import directly using Execute() and it would just work.

But I feel this might require deeper knowledge of Jint to do right.

Option 3: Allow import in scripts, ignoring ECMAScript

The dirty option would be to "ignore" the "scriptOrModule" semantics, and just resolve export and import statements in a different way for the root. For example, export currently binds things on JsModule, but with this option we would have a special Exports object when we're not running within a module. For example:

engine.Execute("export 'test';");
var value = engine.GetDefaultExport().AsString();

This wouldn't be that bad, but it's definitely not what ECMAScript prescribes.

[lahma]

I think all we need is a new entry-point?

var engine = new Engine();

const string script = @"
    export const sqrt = Math.sqrt;
    export function square(x) {
        return x * x;
    }";

var module = new JavaScriptParser(script).ParseModule();
engine.Execute(module);

// Engine.Modules.cs
public Engine Execute(Module module)
{
    // magic happens
}

scriptOrModule is Program in Esprima terms, both Script and Module inherit from it.

[christianrondeau]

That makes perfect sense (it's pretty much option 1 if I got this right but with existing stuff), that also means we don't deal with import/export madness in the main script execution context, at least for a first PR. I'll see where I can go with this, thanks for the pointers!

[christianrondeau]

Good news is I got it working; most of the code was there already.

var module = new JavaScriptParser(@"export const value = 'exported value';").ParseModule();
var jsModule = engine.ExecuteModule(module);

Assert.Equal("exported value", jsModule.ImportValue("value").AsString());

Only two hacks here:

• The JintExportDeclarationStatement calls ModuleEnvironmentRecord.CreateImportBinding, but there's no module import really.
• I created JsModule.ImportValue which returns _context.VariableEnvironment.GetBindingValue(name, true).

That made me realize, export typically only makes sense where you're loading a module, so rather than building on my initial idea (i.e. anything exported becomes magically available for reading), I'm thinking the following could be more in line with how modules work:

var module = new JavaScriptParser(@"export const value = 'exported value';").ParseModule();
engine.DefineModule(module, "my-module");
engine.Execute("import { value } from 'my-module';");

Assert.Equal("exported value", engine.GetValue("value").AsString());

This is still technically invalid since this allows using import from Engine directly (it should throw with Cannot use import statement outside a module) but everything else falls in place nicely. That also means I have to implement import too but I think that's okay.

Once I'm confident enough with the implementation, I'll create a PR for review (unless you see something wrong in what I just said). Let me know if you'd prefer seeing broken code earlier and I'll share my work branch.

[lahma]

Good news is I got it working; most of the code was there already.

Great to hear that you are making progress and the constructs make sense! I was hoping that the engine part for module handling is already quite complete and only needs more runtime behavior implementation, which seems to be the case.

var module = new JavaScriptParser(@"export const value = 'exported value';").ParseModule();
var jsModule = engine.ExecuteModule(module);

Assert.Equal("exported value", jsModule.ImportValue("value").AsString());

ExecuteModule seems like a nice distinction as module code is quite different and has a lot of different semantics on usage. /cc @sebastienros for API review (might change).

Only two hacks here:

• The JintExportDeclarationStatement calls ModuleEnvironmentRecord.CreateImportBinding, but there's no module import really.
• I created JsModule.ImportValue which returns _context.VariableEnvironment.GetBindingValue(name, true).

I think we can revisit the "hacks" later and try to align when the picture gets clearer (it isn't fully clear to me yet).

var module = new JavaScriptParser(@"export const value = 'exported value';").ParseModule();
engine.DefineModule(module, "my-module");
engine.Execute("import { value } from 'my-module';");

Assert.Equal("exported value", engine.GetValue("value").AsString());

The engine API will be crucial part as it will be public, generally everything else should be internal so we can move the cheese as much as we want when we refactor and improve. DefineModule seems to become public here, it's a logical service, but we need to check the API at the end at least.

This is still technically invalid since this allows using import from Engine directly (it should throw with Cannot use import statement outside a module) but everything else falls in place nicely. That also means I have to implement import too but I think that's okay.

Yes it's funny how it would be convenient to use the import in regular script evaluation and being against the spec. I guess this would mean in practice to throw an error about You seem to be running imports / exports, you need to use ExececuteModule, for reasons or something...

Once I'm confident enough with the implementation, I'll create a PR for review (unless you see something wrong in what I just said). Let me know if you'd prefer seeing broken code earlier and I'll share my work branch.

I always like to see broken code 😉 Jokes aside, it would be nice if you'd create a PR with descriptive title to inform others that this is being worked on. If I read properly between the lines you are also going to tackle the import behavior? You can open a PR as draft so it signals it not being ready for review but I'd be happy to go trough it if you'd want to. I'm good at nit-picking about whitespace at least.

[christianrondeau]

Will do, for now, I'm still figuring out the pieces (e.g. I misunderstood what JsModule._importEntries was and went on the wrong track). As soon as I have a path that actually makes sense I'll open a draft PR. I think we might not need a script-level import after all, just a method to access exports directly from a JsModule. I'll do my homework and reach out soon :)

[christianrondeau]

PR created as a draft; I'll move the discussion there :D