-
-
Notifications
You must be signed in to change notification settings - Fork 547
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
Support Esprima.Ast.ExportNamedDeclartion #636
Comments
Import/export should be covered in #430 , I'll close this and we should continue tracking on the specific feature issue. |
It seems #430 was closed, but as far as I can tell it's still impossible to use something like 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 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 :) |
The module runtime behavior is still lacking, the static constructs and services are there, but runtime evaluation is missing some bits. |
I was considering investigating at least two things, so I'm not creating a monster :) My questions:
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 importsimport { 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
} |
@christianrondeau I did small sprint one evening in the past checking what was missing, I've pushed the very WIP, a sketch here: #1051 . So what is needed to examine the spec and how things are supposed to work, in this module related runtime behavior like 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 |
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;
Thanks :) |
[MemberData(nameof(SourceFiles), "language\\module-code", false)]
[MemberData(nameof(SourceFiles), "language\\module-code", true, Skip = "Skipped")] Means that
Might be a problem with full framework.
I've reopened it. So the PR should probably produce a Thanks for helping out! |
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. |
Take everything you need, there might be some errors etc, but whatever helps the effort. |
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 Option 1: Execute, GetValue runs on a scriptThis means, no 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 scriptOrModuleThis would be more in line with ECMAScript but would require much heavier refactoring. Right now, But I feel this might require deeper knowledge of Jint to do right. Option 3: Allow import in scripts, ignoring ECMAScriptThe dirty option would be to "ignore" the "scriptOrModule" semantics, and just resolve engine.Execute("export 'test';");
var value = engine.GetDefaultExport().AsString(); This wouldn't be that bad, but it's definitely not what ECMAScript prescribes. |
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
}
|
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! |
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:
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 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. |
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.
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).
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.
Yes it's funny how it would be convenient to use the
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. |
Will do, for now, I'm still figuring out the pieces (e.g. I misunderstood what |
PR created as a draft; I'll move the discussion there :D |
I want to write a Javascript file like
But this gives me:
This is an ES6 feature.
The text was updated successfully, but these errors were encountered: