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

Scoping and CompiledScript.Execute vs ScriptEngine.Evaluate #213

Closed
lsim opened this issue Jun 9, 2022 · 5 comments
Closed

Scoping and CompiledScript.Execute vs ScriptEngine.Evaluate #213

lsim opened this issue Jun 9, 2022 · 5 comments
Labels

Comments

@lsim
Copy link

lsim commented Jun 9, 2022

Hi Paul

I'm seeing some strange behavior. Just wondering if there is an explanation I'm overlooking.

I have two scripts A and B such as

function main() {
  console.log('Script A');
}
main();

and

function main() {
  console.log('Script B');
}
main();

The key thing is they both rely on the ability to declare the function main in the global scope.

What I am seeing is that running script A via CompiledScript.Execute and then running script B via ScriptEngine.Evaluate results in 'Script A' being logged twice.

Running both via CompiledScript.Execute works as expected.

Running both via ScriptEngine.Evaluate works as expected.

Running script A first via ScriptEngine.Evaluate and then script B via CompiledScript.Execute also works as expected.

It looks as if toplevel scope variables declared via CompiledScript.Execute cannot be overridden via ScriptEngine.Evaluate.

Please note that we still haven't gotten around to updating our engine from v2.2.2, so it is entirely possible that a later version fixes this issue.

@paulbartrum
Copy link
Owner

Yep, that's definitely wrong, later declarations should override earlier ones.

I tried running this code in the latest version (v3.2.6):

static void Main(string[] args)
{
    var engine = new ScriptEngine();
    engine.SetGlobalValue("console", new Jurassic.Library.FirebugConsole(engine));
    engine.Execute(@"
        function main() {
            console.log('Script A');
        }
        main();");
    engine.Evaluate(@"
        function main() {
            console.log('Script B');
        }
        main();");
}

And it worked as expected. Likely it was fixed when I did a big overhaul of how scoping works a couple years back.

Note that this code prints "Script B" twice, and that's also correct (declarations are processed before code):

static void Main(string[] args)
{
    var engine = new ScriptEngine();
    engine.SetGlobalValue("console", new Jurassic.Library.FirebugConsole(engine));
    engine.Execute(@"
        function main() {
            console.log('Script A');
        }
        main();
        function main() {
            console.log('Script B');
        }
        main();");
}

@paulbartrum paulbartrum added the bug label Jun 9, 2022
@lsim
Copy link
Author

lsim commented Jun 9, 2022

Thanks for your speedy reply (as always)!

I somehow thought it would be more work testing it out on 3.2.6. I should have given it a go.

I played around with your code and I'm seeing the same thing you're seeing.

Good to know that it's fixed in 3.2.6. We'll have to put some effort into getting the upgrade under way.

@paulbartrum
Copy link
Owner

If you don't want to update you may be able to clear out the value of "main" in between the two executions, e.g. something like engine.SetGlobalValue("main", Undefined.Value);. I haven't tried it but I guess that would fix it(?)

@paulbartrum paulbartrum closed this as not planned Won't fix, can't repro, duplicate, stale Jun 9, 2022
@lsim
Copy link
Author

lsim commented Jun 10, 2022

Our scripts are entirely dynamic, and we do have some cases where we rely on variables declared in one script being accessed from others, so scope wiping between runs will break things.

I have started the upgrade. Looks doable for now.

@paulbartrum
Copy link
Owner

The code I pasted would only touch the value of "main" and nothing else (so not really "scope wiping"), but if the name of the function is dynamic then yeah that'd be problematic.

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

No branches or pull requests

2 participants