Use script elements to evaluate scripts instead of the vm module

Author: ide

src/HasteModuleLoader/HasteModuleLoader.js

@@ -234,7 +234,7 @@ Loader.prototype._execModule = function(moduleObj) {
   this._isCurrentlyExecutingManualMock = modulePath;
 
   utils.runContentWithLocalBindings(
-    this._environment.runSourceText.bind(this._environment),
+    this._environment,
     moduleContent,
     modulePath,
     moduleLocalBindings

src/HasteModuleLoader/__tests__/HasteModuleLoader-currentTestPath-test.js

@@ -15,4 +15,4 @@ describe('nodeHasteModuleLoader', function() {
             expect(jest.currentTestPath()).toMatch(/currentTestPath-test/);
         });
     });
-});
+});

src/JSDomEnvironment.js

@@ -9,7 +9,8 @@
 
 var FakeTimers = require('./lib/FakeTimers');
 var utils = require('./lib/utils');
-var vm = require('vm');
+
+var USE_JSDOM_EVAL = false;
 
 function JSDomEnvironment(config) {
   // We lazily require jsdom because it takes a good ~.5s to load.
@@ -19,7 +20,13 @@ function JSDomEnvironment(config) {
   // a workerpool parent), this is the best way to ensure we only spend time
   // require()ing this when necessary.
   var jsdom = require('./lib/jsdom-compat');
-  this.document = jsdom.jsdom();
+  this.document = jsdom.jsdom(/* markup */undefined, {
+    resourceLoader: this._fetchExternalResource.bind(this),
+    features: {
+      FetchExternalResources: ['script'],
+      ProcessExternalResources: ['script'],
+    },
+  });
   this.global = this.document.defaultView;
 
   // Node's error-message stack size is limited at 10, but it's pretty useful to
@@ -94,17 +101,40 @@ JSDomEnvironment.prototype.dispose = function() {
 };
 
 /**
- * Evaluates the given source text as if it were in a file with the given name
- * and returns the result.
+ * Evaluates the given source text as if it were in a file with the given name.
+ * This method returns nothing.
  */
 JSDomEnvironment.prototype.runSourceText = function(sourceText, fileName) {
-  // TODO: Stop using the private API and instead configure jsdom with a
-  // resource loader and insert <script src="${filename}"> in the document. The
-  // reason we use vm for now is because the script element technique is slow.
-  return vm.runInContext(sourceText, this.document._ownerDocument._global, {
-    filename: fileName,
-    displayErrors: false,
-  });
+  if (!USE_JSDOM_EVAL) {
+    var vm = require('vm');
+    vm.runInContext(sourceText, this.document._ownerDocument._global, {
+      filename: fileName,
+      displayErrors: false,
+    });
+    return;
+  }
+
+  // We evaluate code by inserting <script src="${filename}"> into the document
+  // and using jsdom's resource loader to simulate serving the source code.
+  this._scriptToServe = sourceText;
+
+  var scriptElement = this.document.createElement('script');
+  scriptElement.src = fileName;
+
+  this.document.head.appendChild(scriptElement);
+  this.document.head.removeChild(scriptElement);
+};
+
+JSDomEnvironment.prototype._fetchExternalResource = function(
+  resource,
+  callback
+) {
+  var content = this._scriptToServe;
+  delete this._scriptToServe;
+  if (content === null || content === undefined) {
+    var error = new Error('Unable to find source for ' + resource.url.href);
+  }
+  callback(error, content);
 };
 
 JSDomEnvironment.prototype.runWithRealTimers = function(cb) {

src/TestRunner.js

@@ -361,7 +361,7 @@ TestRunner.prototype.runTest = function(testFilePath) {
 
     if (config.setupEnvScriptFile) {
       utils.runContentWithLocalBindings(
-        env.runSourceText.bind(env),
+        env,
         utils.readAndPreprocessFileContent(config.setupEnvScriptFile, config),
         config.setupEnvScriptFile,
         {

src/__mocks__/JSDomEnvironment.js

@@ -20,7 +20,7 @@ JSDomEnvironmentMock.mockImplementation(function(config) {
 
 JSDomEnvironmentMock.prototype.runSourceText.mockImplementation(
   function(codeStr, fileName) {
-    return vm.runInNewContext(codeStr, this.global, {
+    vm.runInNewContext(codeStr, this.global, {
       filename: fileName,
       displayErrors: false,
     });

src/jasmineTestRunner/jasmineTestRunner.js

@@ -204,7 +204,7 @@ function jasmineTestRunner(config, environment, moduleLoader, testPath) {
       );
 
       utils.runContentWithLocalBindings(
-        environment.runSourceText.bind(environment),
+        environment,
         setupScriptContent,
         config.setupTestFrameworkScriptFile,
         {

src/lib/utils.js

@@ -32,6 +32,11 @@ var DEFAULT_CONFIG_VALUES = {
   noHighlight: false,
 };
 
+// This shows up in the stack trace when a test file throws an unhandled error
+// when evaluated. Node's require prints Object.<anonymous> when initializing
+// modules, so do the same here solely for visual consistency.
+var EVAL_RESULT_VARIABLE = 'Object.<anonymous>';
+
 function _replaceRootDirTags(rootDir, config) {
   switch (typeof config) {
     case 'object':
@@ -361,21 +366,26 @@ function readAndPreprocessFileContent(filePath, config) {
   return cacheRec.content;
 }
 
-function runContentWithLocalBindings(contextRunner, scriptContent, scriptPath,
+function runContentWithLocalBindings(environment, scriptContent, scriptPath,
                                      bindings) {
   var boundIdents = Object.keys(bindings);
   try {
-    var wrapperFunc = contextRunner(
-      '(function(' + boundIdents.join(',') + '){' +
+    var wrapperScript = 'this["' + EVAL_RESULT_VARIABLE + '"] = ' +
+      'function (' + boundIdents.join(',') + ') {' +
       scriptContent +
-      '\n})',
+      '\n};';
+    environment.runSourceText(
+      wrapperScript,
       scriptPath
     );
   } catch (e) {
     e.message = scriptPath + ': ' + e.message;
     throw e;
   }
 
+  var wrapperFunc = environment.global[EVAL_RESULT_VARIABLE];
+  delete environment.global[EVAL_RESULT_VARIABLE];
+
   var bindingValues = boundIdents.map(function(ident) {
     return bindings[ident];
   });