Clean up JSDomEnvironment

Summary: This cleans up the JSDomEnvironment and makes it actually very simple – it should make it more obvious and easier to create different environments (soon!). I'm also considering to make the `real-timer` use case a separate environment instead of a jest toggle (`useFakeTimers` and `useRealTimers`).

This diff

* Removes `mockSetReadOnlyProperty`, the API for this kind of stuff is `Object.defineProperty` which is almost the same
* Removes setting `navigator.onLine`. Only a single test depended on this and it has a setter to set this property, so that was an easy fix. This shouldn't be mandated by the jsdom environment and should be a user specific setting.
* Removes the script loading mechanism that we never actually used and doesn't work for us. I'm happy with `runInContext` for now, we can change it to something else in the future if we care to.
* Doesn't check for `setImmediate` as its part of the node environment that we support

public

Reviewed By: voideanvalue

Differential Revision: D2603413

fb-gh-sync-id: 085da5bf58e9f7b5645ce7772e73dcaafaff1fae

Author: cpojer

CHANGELOG.md

@@ -6,6 +6,9 @@
  * Example: http://astexplorer.net/#/zrybZ6UvRA
  * Codemod: https://github.com/cpojer/js-codemod/blob/master/transforms/jest-update.js
  * jscodeshift: https://github.com/facebook/jscodeshift
+* Removed `navigator.onLine` and `mockSetReadOnlyProperty` from the global jsdom
+  environment. Use `window.navigator.onLine = true;` in your test setup and
+  `Object.defineProperty` instead.
 
 ## 0.6.1
 

src/JSDomEnvironment.js

@@ -9,26 +9,14 @@
 
 const FakeTimers = require('./lib/FakeTimers');
 const utils = require('./lib/utils');
-
-const USE_JSDOM_EVAL = false;
+const vm = require('vm');
 
 class JSDomEnvironment {
 
   constructor(config) {
-    // We lazily require jsdom because it takes a good ~.5s to load.
-    //
-    // Since this file may be require'd at the top of other files that may/may
-    // not use it (depending on the context -- such as TestRunner.js when
-    // operating as a workerpool parent), this is the best way to ensure we
-    // only spend time require()ing this when necessary.
-    const jsdom = require('jsdom');
-    this.document = jsdom.jsdom(/* markup */undefined, {
+    // lazy require
+    this.document = require('jsdom').jsdom(/* markup */undefined, {
       url: config.testURL,
-      resourceLoader: this._fetchExternalResource.bind(this),
-      features: {
-        FetchExternalResources: ['script'],
-        ProcessExternalResources: ['script'],
-      },
     });
     this.global = this.document.defaultView;
 
@@ -39,80 +27,23 @@ class JSDomEnvironment {
     // Forward some APIs
     this.global.Buffer = Buffer;
     this.global.process = process;
-
-    // Setup defaults for navigator.onLine
-    this.global.navigator.onLine = true;
-
-    if (typeof setImmediate === 'function') {
-      this.global.setImmediate = setImmediate;
-      this.global.clearImmediate = clearImmediate;
-    }
+    this.global.setImmediate = setImmediate;
+    this.global.clearImmediate = clearImmediate;
 
     this.fakeTimers = new FakeTimers(this.global);
 
-    // I kinda wish tests just did this manually rather than relying on a
-    // helper function to do it, but I'm keeping it for backward compat reasons
-    // while we get jest deployed internally. Then we can look into removing it.
-    //
-    // #3376754
-    if (!this.global.hasOwnProperty('mockSetReadOnlyProperty')) {
-      this.global.mockSetReadOnlyProperty = function(obj, property, value) {
-        obj.__defineGetter__(property, function() {
-          return value;
-        });
-      };
-    }
-
-    // Apply any user-specified global vars
-    const globalValues = utils.deepCopy(config.globals);
-    for (const customGlobalKey in globalValues) {
-      // Always deep-copy objects so isolated test environments can't share
-      // memory
-      this.global[customGlobalKey] = globalValues[customGlobalKey];
-    }
+    Object.assign(this.global, utils.deepCopy(config.globals));
   }
 
   dispose() {
     this.global.close();
   }
 
-  /**
-   * Evaluates the given source text as if it were in a file with the given
-   * name. This method returns nothing.
-   */
-  runSourceText(sourceText, fileName) {
-    if (!USE_JSDOM_EVAL) {
-      const 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;
-
-    const scriptElement = this.document.createElement('script');
-    scriptElement.src = fileName;
-
-    this.document.head.appendChild(scriptElement);
-    this.document.head.removeChild(scriptElement);
-  }
-
-  _fetchExternalResource(
-    resource,
-    callback
-  ) {
-    const content = this._scriptToServe;
-    let error;
-    delete this._scriptToServe;
-    if (content === null || content === undefined) {
-      error = new Error('Unable to find source for ' + resource.url.href);
-    }
-    callback(error, content);
+  runSourceText(sourceText, filename) {
+    vm.runInContext(sourceText, this.document._ownerDocument._global, {
+      filename,
+      displayErrors: false,
+    });
   }
 
   runWithRealTimers(cb) {