search

mozilla / positron

a experimental, Electron-compatible runtime on top of Gecko
Other
562 stars 64 forks source link

Load modules in their own scopes with evalInSandbox. #97

Closed brendandahl closed 8 years ago

brendandahl commented 8 years ago

Fixes #93 an #45

This solution doesn't seem quite as clean as using the built in loadSubScript and I wonder about the performance implications of loading the entire script into a string and evaling it.

We could instead try to fix the DOM bindings issue I mentioned in https://github.com/mozilla/positron/issues/45#issuecomment-228914035

Thoughts?

mykmelez commented 8 years ago

This solution doesn't seem quite as clean as using the built in loadSubScript and I wonder about the performance implications of loading the entire script into a string and evaling it.

Indeed! However, mozIJSSubScriptLoader.loadSubScript with a targetObj also has perf implications.

We could instead try to fix the DOM bindings issue I mentioned in #45 (comment)

Let's do both: land this to fix those issues in the short term, then figure out a fix for the DOM bindings issue and switch back to loadSubScript in conjunction with it.

mykmelez commented 8 years ago

Let's do both: land this to fix those issues in the short term, then figure out a fix for the DOM bindings issue and switch back to loadSubScript in conjunction with it.

I merged this, which closed #93, but it left #45 open. We can use #45 to track the DOM bindings issue or close it as well and file another issue for that; either way is fine with me.