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

chore: migrate jest-runtime to TypeScript #7964

Merged
merged 16 commits into from
Feb 25, 2019
Merged

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Feb 23, 2019

Summary

I've also messed about with the types of a couple of other packages to make things pass

Built diff:

diff --git c/packages/jest-runtime/build/cli/args.js w/packages/jest-runtime/build/cli/args.js
index 1951c9de2..9419c6099 100644
--- c/packages/jest-runtime/build/cli/args.js
+++ w/packages/jest-runtime/build/cli/args.js
@@ -10,8 +10,6 @@ exports.options = exports.usage = void 0;
  *
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
- *
- *
  */
 const usage = 'Usage: $0 [--config=<pathToConfigFile>] <file>';
 exports.usage = usage;
diff --git c/packages/jest-runtime/build/cli/index.js w/packages/jest-runtime/build/cli/index.js
index 3247eda92..ea30f53d9 100644
--- c/packages/jest-runtime/build/cli/index.js
+++ w/packages/jest-runtime/build/cli/index.js
@@ -5,30 +5,30 @@ Object.defineProperty(exports, '__esModule', {
 });
 exports.run = run;
 
-function _chalk() {
-  const data = _interopRequireDefault(require('chalk'));
+function _os() {
+  const data = _interopRequireDefault(require('os'));
 
-  _chalk = function _chalk() {
+  _os = function _os() {
     return data;
   };
 
   return data;
 }
 
-function _os() {
-  const data = _interopRequireDefault(require('os'));
+function _path() {
+  const data = _interopRequireDefault(require('path'));
 
-  _os = function _os() {
+  _path = function _path() {
     return data;
   };
 
   return data;
 }
 
-function _path() {
-  const data = _interopRequireDefault(require('path'));
+function _chalk() {
+  const data = _interopRequireDefault(require('chalk'));
 
-  _path = function _path() {
+  _chalk = function _chalk() {
     return data;
   };
 
@@ -85,6 +85,8 @@ function _jestConfig() {
   return data;
 }
 
+var _version = require('../version');
+
 var args = _interopRequireWildcard(require('./args'));
 
 function _interopRequireWildcard(obj) {
@@ -148,8 +150,6 @@ function _defineProperty(obj, key, value) {
   return obj;
 }
 
-const VERSION = require('../../package.json').version;
-
 function run(cliArgv, cliInfo) {
   const realFs = require('fs');
 
@@ -182,7 +182,7 @@ function run(cliArgv, cliInfo) {
   }
 
   if (argv.version) {
-    console.log(`v${VERSION}\n`);
+    console.log(`v${_version.VERSION}\n`);
     return;
   }
 
@@ -198,7 +198,7 @@ function run(cliArgv, cliInfo) {
 
   if (argv.debug) {
     const info = cliInfo ? ', ' + cliInfo.join(', ') : '';
-    console.log(`Using Jest Runtime v${VERSION}${info}`);
+    console.log(`Using Jest Runtime v${_version.VERSION}${info}`);
   }
 
   const options = (0, _jestConfig().readConfig)(argv, root);
@@ -216,7 +216,6 @@ function run(cliArgv, cliInfo) {
     watchman: globalConfig.watchman
   })
     .then(hasteMap => {
-      /* $FlowFixMe */
       const Environment = require(config.testEnvironment);
 
       const environment = new Environment(config);
@@ -225,8 +224,16 @@ function run(cliArgv, cliInfo) {
         'console',
         new (_jestUtil()).Console(process.stdout, process.stderr)
       );
-      environment.global.jestProjectConfig = config;
-      environment.global.jestGlobalConfig = globalConfig;
+      (0, _jestUtil().setGlobal)(
+        environment.global,
+        'jestProjectConfig',
+        config
+      );
+      (0, _jestUtil().setGlobal)(
+        environment.global,
+        'jestGlobalConfig',
+        globalConfig
+      );
       const runtime = new Runtime(config, environment, hasteMap.resolver);
       runtime.requireModule(filePath);
     })
diff --git c/packages/jest-runtime/build/index.js w/packages/jest-runtime/build/index.js
index d510fd126..0a309a874 100644
--- c/packages/jest-runtime/build/index.js
+++ w/packages/jest-runtime/build/index.js
@@ -115,9 +115,10 @@ function _interopRequireDefault(obj) {
  *
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
- *
- *
  */
+const testTimeoutSymbol = Symbol.for('TEST_TIMEOUT_SYMBOL');
+const retryTimesSymbol = Symbol.for('RETRY_TIMES');
+
 const NODE_MODULES = _path().default.sep + 'node_modules' + _path().default.sep;
 
 const getModuleNameMapper = config => {
@@ -141,7 +142,7 @@ class Runtime {
     this._cacheFS = cacheFS || Object.create(null);
     this._config = config;
     this._coverageOptions = coverageOptions || {
-      changedFiles: null,
+      changedFiles: undefined,
       collectCoverage: false,
       collectCoverageFrom: [],
       collectCoverageOnlyFrom: null
@@ -195,19 +196,10 @@ class Runtime {
         this.requireModule(config.setupFiles[i]);
       }
     }
-  }
+  } // TODO: Can this be `static shouldInstrument = shouldInstrument;`?
 
   static shouldInstrument(filename, options, config) {
-    return (0, _transform().shouldInstrument)(
-      filename,
-      {
-        changedFiles: options.changedFiles,
-        collectCoverage: options.collectCoverage,
-        collectCoverageFrom: options.collectCoverageFrom,
-        collectCoverageOnlyFrom: options.collectCoverageOnlyFrom
-      },
-      config
-    );
+    return (0, _transform().shouldInstrument)(filename, options, config);
   }
 
   static createContext(config, options) {
@@ -242,7 +234,7 @@ class Runtime {
     const ignorePattern =
       ignorePatternParts.length > 0
         ? new RegExp(ignorePatternParts.join('|'))
-        : null;
+        : undefined;
     return new (_jestHasteMap()).default({
       cacheDirectory: config.cacheDirectory,
       computeSha1: config.haste.computeSha1,
@@ -357,7 +349,6 @@ class Runtime {
           )
         );
       } else if (_path().default.extname(modulePath) === '.node') {
-        // $FlowFixMe
         localModule.exports = require(modulePath);
       } else {
         // Only include the fromPath if a moduleName is given. Else treat as root.
@@ -517,10 +508,11 @@ class Runtime {
           const globalMock = envGlobal[key];
 
           if (
-            (typeof globalMock === 'object' && globalMock !== null) ||
-            typeof globalMock === 'function'
+            ((typeof globalMock === 'object' && globalMock !== null) ||
+              typeof globalMock === 'function') &&
+            globalMock._isMockFunction === true
           ) {
-            globalMock._isMockFunction === true && globalMock.mockClear();
+            globalMock.mockClear();
           }
         });
       }
@@ -588,13 +580,15 @@ class Runtime {
     return to ? this._resolver.resolveModule(from, to) : from;
   }
 
-  _requireResolve(from, moduleName, {paths} = {}) {
+  _requireResolve(from, moduleName, options = {}) {
     if (moduleName == null) {
       throw new Error(
         'The first argument to require.resolve must be a string. Received null or undefined.'
       );
     }
 
+    const paths = options.paths;
+
     if (paths) {
       var _iteratorNormalCompletion = true;
       var _didIteratorError = false;
@@ -697,18 +691,14 @@ class Runtime {
     const dirname = _path().default.dirname(filename);
 
     localModule.children = [];
-    Object.defineProperty(
-      localModule,
-      'parent', // https://github.com/facebook/flow/issues/285#issuecomment-270810619
-      {
-        enumerable: true,
+    Object.defineProperty(localModule, 'parent', {
+      enumerable: true,
 
-        get() {
-          const key = from || '';
-          return moduleRegistry[key] || null;
-        }
+      get() {
+        const key = from || '';
+        return moduleRegistry[key] || null;
       }
-    );
+    });
     localModule.paths = this._resolver.getModulePaths(dirname);
     Object.defineProperty(localModule, 'require', {
       value: this._createRequireImplementation(localModule, options)
@@ -757,10 +747,7 @@ class Runtime {
       dirname, // __dirname
       filename, // __filename
       this._environment.global, // global object
-      this._createJestObjectFor(
-        filename, // $FlowFixMe
-        localModule.require
-      ), // jest object
+      this._createJestObjectFor(filename, localModule.require), // jest object
       ...extraGlobals.map(globalVariable => {
         if (this._environment.global[globalVariable]) {
           return this._environment.global[globalVariable];
@@ -779,7 +766,7 @@ class Runtime {
   _requireCoreModule(moduleName) {
     if (moduleName === 'process') {
       return this._environment.global.process;
-    } // $FlowFixMe
+    }
 
     return require(moduleName);
   }
@@ -897,6 +884,7 @@ class Runtime {
   }
 
   _createRequireImplementation(from, options) {
+    // TODO: Should not be `any`, this isn't type safe. Should be `LocalModuleRequire`
     const moduleRequire =
       options && options.isInternalModule
         ? moduleName => this.requireInternalModule(from.filename, moduleName)
@@ -904,7 +892,7 @@ class Runtime {
     moduleRequire.cache = Object.create(null);
     moduleRequire.extensions = Object.create(null);
     moduleRequire.requireActual = this.requireActual.bind(this, from.filename);
-    moduleRequire.requireMock = this.requireMock.bind(this, from.filename);
+    moduleRequire.requireMock = this.requireMock.bind(this, from.filename); // TODO: somehow avoid having to type the arguments - they should come from `NodeRequire.resolve`
 
     moduleRequire.resolve = (moduleName, options) =>
       this._requireResolve(from.filename, moduleName, options);
@@ -1028,16 +1016,19 @@ class Runtime {
     const spyOn = this._moduleMocker.spyOn.bind(this._moduleMocker);
 
     const setTimeout = timeout => {
-      this._environment.global.jasmine
-        ? (this._environment.global.jasmine._DEFAULT_TIMEOUT_INTERVAL = timeout)
-        : (this._environment.global[
-            Symbol.for('TEST_TIMEOUT_SYMBOL')
-          ] = timeout);
+      if (this._environment.global.jasmine) {
+        this._environment.global.jasmine._DEFAULT_TIMEOUT_INTERVAL = timeout;
+      } else {
+        // @ts-ignore: https://github.com/Microsoft/TypeScript/issues/24587
+        this._environment.global[testTimeoutSymbol] = timeout;
+      }
+
       return jestObject;
     };
 
     const retryTimes = numTestRetries => {
-      this._environment.global[Symbol.for('RETRY_TIMES')] = numTestRetries;
+      // @ts-ignore: https://github.com/Microsoft/TypeScript/issues/24587
+      this._environment.global[retryTimesSymbol] = numTestRetries;
       return jestObject;
     };
 
diff --git c/packages/jest-runtime/build/types.js w/packages/jest-runtime/build/types.js
new file mode 100644
index 000000000..ad9a93a7c
--- /dev/null
+++ w/packages/jest-runtime/build/types.js
@@ -0,0 +1 @@
+'use strict';
diff --git c/packages/jest-runtime/build/version.js w/packages/jest-runtime/build/version.js
new file mode 100644
index 000000000..6df9698ea
--- /dev/null
+++ w/packages/jest-runtime/build/version.js
@@ -0,0 +1,16 @@
+'use strict';
+
+Object.defineProperty(exports, '__esModule', {
+  value: true
+});
+exports.VERSION = void 0;
+
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+const VERSION = require('../package.json').version;
+
+exports.VERSION = VERSION;

Test plan

Green CI

@@ -0,0 +1,22 @@
{
"name": "@jest/environment",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly so I could reuse jest-mock types
It'll be useful for all test environments, though

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @lirbank relevant for your migrations

@SimenB SimenB force-pushed the ts-jest-runtime branch 2 times, most recently from 01b2af6 to 5f6d79a Compare February 23, 2019 16:16
@jeysal
Copy link
Contributor

jeysal commented Feb 23, 2019

Pushed a fix for the remaining partial Module type errors to https://github.com/jeysal/jest/tree/ts-jest-runtime :)
(jeysal@c7a7d0b)

@SimenB SimenB marked this pull request as ready for review February 23, 2019 19:35
@SimenB
Copy link
Member Author

SimenB commented Feb 23, 2019

Thanks @jeysal, cherry picked!

This is ready for review now, if anyone feels up to it.

I added the typings for the Jest object to a new package @jest/environment. This will eventually be part of the typings for people who use Jest, which is why I've added docs to the methods exposed. However, this is the wrong place for it (my thinking was that @jest/environment just exports the interface implemented by all test environments), but I haven't figured out where I want it to live.

@codecov-io
Copy link

codecov-io commented Feb 23, 2019

Codecov Report

Merging #7964 into master will decrease coverage by 0.17%.
The diff coverage is 61.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7964      +/-   ##
==========================================
- Coverage   64.78%   64.61%   -0.18%     
==========================================
  Files         256      257       +1     
  Lines       10045    10067      +22     
  Branches     1466     1556      +90     
==========================================
- Hits         6508     6505       -3     
- Misses       3218     3224       +6     
- Partials      319      338      +19
Impacted Files Coverage Δ
packages/jest-transform/src/shouldInstrument.ts 88.23% <ø> (ø) ⬆️
packages/jest-transform/src/ScriptTransformer.ts 81.28% <ø> (ø) ⬆️
...circus/src/legacy-code-todo-rewrite/jestAdapter.ts 0% <ø> (ø) ⬆️
packages/jest-runtime/src/helpers.ts 88.23% <ø> (ø)
packages/jest-resolve/src/index.ts 40.62% <ø> (ø) ⬆️
packages/jest-runtime/src/version.ts 100% <100%> (ø)
packages/jest-runtime/src/index.ts 68.9% <60.6%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fddf439...95e4b6a. Read the comment docs.

@SimenB
Copy link
Member Author

SimenB commented Feb 25, 2019

This is a bit painful to keep up to date with master, so I'll merge when CI is green. Feel free to review this after the fact, and I can open up a new PR with fixes proposed

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants