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

Move vm.runInThisContext to require.exec #849

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AMKohn
Copy link

@AMKohn AMKohn commented Aug 30, 2015

In split contexts, like NW.js', vm.runInThisContext evaluates modules in the wrong context. And, due to its location in the code, this behavior is impossible to override.

This commit moves the call to require.exec, where it's overridable, and changes require.makeNodeWrapper to use global.requirejsVars so it doesn't reference a non-existent object when run in NW.js' context.

With this change require can be used in NW.js when exec is overridden like this (although there's a bug with vm in NW.js at the moment so eval has to be used instead):

var vm = require("vm"),
    context = vm.createContext(window);

requirejs.exec = function(text, realPath) {
    text = requirejs.makeNodeWrapper(text);

    return vm.runInContext(text, context, realPath);
};

In split contexts, like NW.js, runInThisContext evaluates modules in the wrong context.  And, due to its location, this behavior is impossible to override.

This commit moves the call to require.exec, which is overridable, and changes makeNodeWrapper to use `global.requirejsVars` instead of just requirejsVars.
@digitalkaoz
Copy link

Can someone merge this?

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

Successfully merging this pull request may close these issues.

2 participants