chore: migrate jest-worker to TypeScript #7853

SimenB · 2019-02-10T14:07:27Z

Summary

I left the tests alone as there were way too many typer errors for the effort I wanted to put into it :P

At some point we can probably make the types smarter (having it infer exports from the file coming in, filtering by exposedMethods etc). My hope is that someone will contribute that as a PR at some point 😀

built diff:

diff --git i/packages/jest-worker/build/Farm.js w/packages/jest-worker/build/Farm.js
index ed652e867..f16fe5df8 100644
--- i/packages/jest-worker/build/Farm.js
+++ w/packages/jest-worker/build/Farm.js
@@ -1,11 +1,3 @@
-/**
- * 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.
- *
- *
- */
 'use strict';
 
 Object.defineProperty(exports, '__esModule', {
@@ -15,6 +7,12 @@ exports.default = void 0;
 
 var _types = require('./types');
 
+/**
+ * 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.
+ */
 class Farm {
   constructor(numOfWorkers, callback, computeWorkerKey) {
     this._callback = callback;
@@ -38,7 +36,7 @@ class Farm {
       let hash = null;
 
       if (computeWorkerKey) {
-        hash = computeWorkerKey.apply(this, [method].concat(args));
+        hash = computeWorkerKey.call(this, method, ...args);
         worker = hash == null ? null : this._cacheKeys[hash];
       }
 
@@ -74,7 +72,7 @@ class Farm {
     let queueHead = this._queue[workerId];
 
     while (queueHead && queueHead.request[1]) {
-      queueHead = queueHead.next;
+      queueHead = queueHead.next || null;
     }
 
     this._queue[workerId] = queueHead;
diff --git i/packages/jest-worker/build/WorkerPool.js w/packages/jest-worker/build/WorkerPool.js
index 5ab4dada6..16ed8908b 100644
--- i/packages/jest-worker/build/WorkerPool.js
+++ w/packages/jest-worker/build/WorkerPool.js
@@ -1,11 +1,3 @@
-/**
- * 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.
- *
- *
- */
 'use strict';
 
 Object.defineProperty(exports, '__esModule', {
@@ -19,6 +11,12 @@ function _interopRequireDefault(obj) {
   return obj && obj.__esModule ? obj : {default: obj};
 }
 
+/**
+ * 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 canUseWorkerThreads = () => {
   try {
     // $FlowFixMe: Flow doesn't know about experimental APIs
diff --git i/packages/jest-worker/build/base/BaseWorkerPool.js w/packages/jest-worker/build/base/BaseWorkerPool.js
index 8f83a8808..eefd02c84 100644
--- i/packages/jest-worker/build/base/BaseWorkerPool.js
+++ w/packages/jest-worker/build/base/BaseWorkerPool.js
@@ -1,11 +1,3 @@
-/**
- * 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.
- *
- *
- */
 'use strict';
 
 Object.defineProperty(exports, '__esModule', {
@@ -13,20 +5,20 @@ Object.defineProperty(exports, '__esModule', {
 });
 exports.default = void 0;
 
-function _mergeStream() {
-  const data = _interopRequireDefault(require('merge-stream'));
+function _path() {
+  const data = _interopRequireDefault(require('path'));
 
-  _mergeStream = function _mergeStream() {
+  _path = function _path() {
     return data;
   };
 
   return data;
 }
 
-function _path() {
-  const data = _interopRequireDefault(require('path'));
+function _mergeStream() {
+  const data = _interopRequireDefault(require('merge-stream'));
 
-  _path = function _path() {
+  _mergeStream = function _mergeStream() {
     return data;
   };
 
@@ -39,6 +31,13 @@ function _interopRequireDefault(obj) {
   return obj && obj.__esModule ? obj : {default: obj};
 }
 
+/**
+ * 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.
+ */
+
 /* istanbul ignore next */
 const emptyMethod = () => {};
 
@@ -100,7 +99,7 @@ class BaseWorkerPool {
     return this._workers[workerId];
   }
 
-  createWorker(workerOptions) {
+  createWorker(_workerOptions) {
     throw Error('Missing method createWorker in WorkerPool');
   }
 
diff --git i/packages/jest-worker/build/index.js w/packages/jest-worker/build/index.js
index 45fc7702a..73ba44544 100644
--- i/packages/jest-worker/build/index.js
+++ w/packages/jest-worker/build/index.js
@@ -1,11 +1,3 @@
-/**
- * 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.
- *
- *
- */
 'use strict';
 
 Object.defineProperty(exports, '__esModule', {
@@ -67,15 +59,15 @@ function getExposedMethods(workerPath, options) {
   let exposedMethods = options.exposedMethods; // If no methods list is given, try getting it by auto-requiring the module.
 
   if (!exposedMethods) {
-    // $FlowFixMe: This has to be a dynamic require.
     const module = require(workerPath);
 
     exposedMethods = Object.keys(module).filter(
+      // @ts-ignore: no index
       name => typeof module[name] === 'function'
     );
 
     if (typeof module === 'function') {
-      exposedMethods.push('default');
+      exposedMethods = [...exposedMethods, 'default'];
     }
   }
 
@@ -110,6 +102,7 @@ function getExposedMethods(workerPath, options) {
 class JestWorker {
   constructor(workerPath, options) {
     this._options = _objectSpread({}, options);
+    this._ending = false;
     const workerPoolOptions = {
       enableWorkerThreads: this._options.enableWorkerThreads || false,
       forkOptions: this._options.forkOptions || {},
@@ -119,9 +112,17 @@ class JestWorker {
         Math.max(_os().default.cpus().length - 1, 1),
       setupArgs: this._options.setupArgs || []
     };
-    this._workerPool = this._options.WorkerPool
-      ? new this._options.WorkerPool(workerPath, workerPoolOptions)
-      : new _WorkerPool.default(workerPath, workerPoolOptions);
+
+    if (this._options.WorkerPool) {
+      // @ts-ignore: constructor target any?
+      this._workerPool = new this._options.WorkerPool(
+        workerPath,
+        workerPoolOptions
+      );
+    } else {
+      this._workerPool = new _WorkerPool.default(workerPath, workerPoolOptions);
+    }
+
     this._farm = new _Farm.default(
       workerPoolOptions.numWorkers,
       this._workerPool.send.bind(this._workerPool),
@@ -139,7 +140,7 @@ class JestWorker {
 
       if (this.constructor.prototype.hasOwnProperty(name)) {
         throw new TypeError('Cannot define a method called ' + name);
-      } // $FlowFixMe: dynamic extension of the class instance is expected.
+      } // @ts-ignore: dynamic extension of the class instance is expected.
 
       this[name] = this._callFunctionWithArgs.bind(this, name);
     });
diff --git i/packages/jest-worker/build/types.js w/packages/jest-worker/build/types.js
index 9ac9874cf..eb8beb09d 100644
--- i/packages/jest-worker/build/types.js
+++ w/packages/jest-worker/build/types.js
@@ -1,19 +1,18 @@
+'use strict';
+
+Object.defineProperty(exports, '__esModule', {
+  value: true
+});
+exports.PARENT_MESSAGE_SETUP_ERROR = exports.PARENT_MESSAGE_CLIENT_ERROR = exports.PARENT_MESSAGE_OK = exports.CHILD_MESSAGE_END = exports.CHILD_MESSAGE_CALL = exports.CHILD_MESSAGE_INITIALIZE = 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.
- *
- *
  */
-'use strict'; // Because of the dynamic nature of a worker communication process, all messages
+// Because of the dynamic nature of a worker communication process, all messages
 // coming from any of the other processes cannot be typed. Thus, many types
-// include "any" as a flow type, which is (unfortunately) correct here.
-
-Object.defineProperty(exports, '__esModule', {
-  value: true
-});
-exports.PARENT_MESSAGE_SETUP_ERROR = exports.PARENT_MESSAGE_CLIENT_ERROR = exports.PARENT_MESSAGE_OK = exports.CHILD_MESSAGE_END = exports.CHILD_MESSAGE_CALL = exports.CHILD_MESSAGE_INITIALIZE = void 0;
 const CHILD_MESSAGE_INITIALIZE = 0;
 exports.CHILD_MESSAGE_INITIALIZE = CHILD_MESSAGE_INITIALIZE;
 const CHILD_MESSAGE_CALL = 1;
@@ -27,4 +26,5 @@ exports.PARENT_MESSAGE_CLIENT_ERROR = PARENT_MESSAGE_CLIENT_ERROR;
 const PARENT_MESSAGE_SETUP_ERROR = 2;
 exports.PARENT_MESSAGE_SETUP_ERROR = PARENT_MESSAGE_SETUP_ERROR;
 
+// Option objects.
 const EventEmitter = require('events');
diff --git i/packages/jest-worker/build/workers/ChildProcessWorker.js w/packages/jest-worker/build/workers/ChildProcessWorker.js
index 3252e6faa..6abdf5721 100644
--- i/packages/jest-worker/build/workers/ChildProcessWorker.js
+++ w/packages/jest-worker/build/workers/ChildProcessWorker.js
@@ -1,11 +1,3 @@
-/**
- * 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.
- *
- *
- */
 'use strict';
 
 Object.defineProperty(exports, '__esModule', {
@@ -23,8 +15,6 @@ function _child_process() {
   return data;
 }
 
-var _types = require('../types');
-
 function _supportsColor() {
   const data = _interopRequireDefault(require('supports-color'));
 
@@ -35,6 +25,8 @@ function _supportsColor() {
   return data;
 }
 
+var _types = require('../types');
+
 function _interopRequireDefault(obj) {
   return obj && obj.__esModule ? obj : {default: obj};
 }
@@ -104,6 +96,7 @@ class ChildProcessWorker {
 
     const child = _child_process().default.fork(
       require.resolve('./processChild'),
+      [],
       _objectSpread(
         {
           cwd: process.cwd(),
@@ -111,7 +104,7 @@ class ChildProcessWorker {
             {},
             process.env,
             {
-              JEST_WORKER_ID: this._options.workerId
+              JEST_WORKER_ID: String(this._options.workerId)
             },
             forceColor
           ),
@@ -150,10 +143,7 @@ class ChildProcessWorker {
     }
   }
 
-  onMessage(
-    response
-    /* Should be ParentMessage */
-  ) {
+  onMessage(response) {
     let error;
 
     switch (response[0]) {
@@ -166,16 +156,16 @@ class ChildProcessWorker {
         error = response[4];
 
         if (error != null && typeof error === 'object') {
-          const extra = error;
+          const extra = error; // @ts-ignore: no index
+
           const NativeCtor = global[response[1]];
           const Ctor = typeof NativeCtor === 'function' ? NativeCtor : Error;
-          error = new Ctor(response[2]); // $FlowFixMe: adding custom properties to errors.
-
+          error = new Ctor(response[2]);
           error.type = response[1];
           error.stack = response[3];
 
           for (const key in extra) {
-            // $FlowFixMe: adding custom properties to errors.
+            // @ts-ignore: adding custom properties to errors.
             error[key] = extra[key];
           }
         }
@@ -185,7 +175,7 @@ class ChildProcessWorker {
         break;
 
       case _types.PARENT_MESSAGE_SETUP_ERROR:
-        error = new Error('Error when calling setup: ' + response[2]); // $FlowFixMe: adding custom properties to errors.
+        error = new Error('Error when calling setup: ' + response[2]); // @ts-ignore: adding custom properties to errors.
 
         error.type = response[1];
         error.stack = response[3];
diff --git i/packages/jest-worker/build/workers/NodeThreadsWorker.js w/packages/jest-worker/build/workers/NodeThreadsWorker.js
index 5a08a8972..d91ed0aa8 100644
--- i/packages/jest-worker/build/workers/NodeThreadsWorker.js
+++ w/packages/jest-worker/build/workers/NodeThreadsWorker.js
@@ -1,11 +1,3 @@
-/**
- * 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.
- *
- *
- */
 'use strict';
 
 Object.defineProperty(exports, '__esModule', {
@@ -13,8 +5,6 @@ Object.defineProperty(exports, '__esModule', {
 });
 exports.default = void 0;
 
-var _types = require('../types');
-
 function _path() {
   const data = _interopRequireDefault(require('path'));
 
@@ -25,6 +15,18 @@ function _path() {
   return data;
 }
 
+function _worker_threads() {
+  const data = require('worker_threads');
+
+  _worker_threads = function _worker_threads() {
+    return data;
+  };
+
+  return data;
+}
+
+var _types = require('../types');
+
 function _interopRequireDefault(obj) {
   return obj && obj.__esModule ? obj : {default: obj};
 }
@@ -61,10 +63,6 @@ function _defineProperty(obj, key, value) {
   return obj;
 }
 
-// $FlowFixMe: Flow doesn't know about experimental features of Node
-const _require = require('worker_threads'),
-  Worker = _require.Worker;
-
 class ExperimentalWorker {
   constructor(options) {
     this._options = options;
@@ -72,7 +70,7 @@ class ExperimentalWorker {
   }
 
   initialize() {
-    this._worker = new Worker(
+    this._worker = new (_worker_threads()).Worker(
       _path().default.resolve(__dirname, './threadChild.js'),
       {
         eval: false,
@@ -82,7 +80,7 @@ class ExperimentalWorker {
           {
             cwd: process.cwd(),
             env: _objectSpread({}, process.env, {
-              JEST_WORKER_ID: this._options.workerId
+              JEST_WORKER_ID: String(this._options.workerId)
             }),
             // Suppress --debug / --inspect flags while preserving others (like --harmony).
             execArgv: process.execArgv.filter(
@@ -124,10 +122,7 @@ class ExperimentalWorker {
     }
   }
 
-  onMessage(
-    response
-    /* Should be ParentMessage */
-  ) {
+  onMessage(response) {
     let error;
 
     switch (response[0]) {
@@ -140,16 +135,16 @@ class ExperimentalWorker {
         error = response[4];
 
         if (error != null && typeof error === 'object') {
-          const extra = error;
+          const extra = error; // @ts-ignore: no index
+
           const NativeCtor = global[response[1]];
           const Ctor = typeof NativeCtor === 'function' ? NativeCtor : Error;
-          error = new Ctor(response[2]); // $FlowFixMe: adding custom properties to errors.
-
+          error = new Ctor(response[2]);
           error.type = response[1];
           error.stack = response[3];
 
           for (const key in extra) {
-            // $FlowFixMe: adding custom properties to errors.
+            // @ts-ignore: no index
             error[key] = extra[key];
           }
         }
@@ -159,7 +154,7 @@ class ExperimentalWorker {
         break;
 
       case _types.PARENT_MESSAGE_SETUP_ERROR:
-        error = new Error('Error when calling setup: ' + response[2]); // $FlowFixMe: adding custom properties to errors.
+        error = new Error('Error when calling setup: ' + response[2]); // @ts-ignore: adding custom properties to errors.
 
         error.type = response[1];
         error.stack = response[3];
diff --git i/packages/jest-worker/build/workers/processChild.js w/packages/jest-worker/build/workers/processChild.js
index 409e0c673..222242c86 100644
--- i/packages/jest-worker/build/workers/processChild.js
+++ w/packages/jest-worker/build/workers/processChild.js
@@ -1,11 +1,3 @@
-/**
- * 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.
- *
- *
- */
 'use strict';
 
 var _types = require('../types');
@@ -118,7 +110,6 @@ function reportError(error, type) {
 }
 
 function end() {
-  // $FlowFixMe: This has to be a dynamic require.
   const main = require(file);
 
   if (!main.teardown) {
@@ -134,7 +125,6 @@ function exitProcess() {
 }
 
 function execMethod(method, args) {
-  // $FlowFixMe: This has to be a dynamic require.
   const main = require(file);
 
   let fn;
diff --git i/packages/jest-worker/build/workers/threadChild.js w/packages/jest-worker/build/workers/threadChild.js
index 346510ed0..941c69992 100644
--- i/packages/jest-worker/build/workers/threadChild.js
+++ w/packages/jest-worker/build/workers/threadChild.js
@@ -1,15 +1,5 @@
-/**
- * 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.
- *
- *
- */
 'use strict';
 
-var _types = require('../types');
-
 function _worker_threads() {
   const data = require('worker_threads');
 
@@ -20,6 +10,8 @@ function _worker_threads() {
   return data;
 }
 
+var _types = require('../types');
+
 function _objectSpread(target) {
   for (var i = 1; i < arguments.length; i++) {
     var source = arguments[i] != null ? arguments[i] : {};
@@ -55,11 +47,6 @@ function _defineProperty(obj, key, value) {
 let file = null;
 let setupArgs = [];
 let initialized = false;
-/* eslint-disable import/no-unresolved */
-// $FlowFixMe: Flow doesn't support experimental node modules
-
-/* eslint-enable import/no-unresolved */
-
 /**
  * This file is a small bootstrapper for workers. It sets up the communication
  * between the worker and the parent process, interpreting parent messages and
@@ -73,6 +60,7 @@ let initialized = false;
  * If an invalid message is detected, the child will exit (by throwing) with a
  * non-zero exit code.
  */
+
 _worker_threads().parentPort.on('message', request => {
   switch (request[0]) {
     case _types.CHILD_MESSAGE_INITIALIZE:
@@ -126,13 +114,12 @@ function reportError(error, type) {
     type,
     error.constructor && error.constructor.name,
     error.message,
-    error.stack, // $FlowFixMe: this is safe to just inherit from Object.
+    error.stack,
     typeof error === 'object' ? _objectSpread({}, error) : error
   ]);
 }
 
 function end() {
-  // $FlowFixMe: This has to be a dynamic require.
   const main = require(file);
 
   if (!main.teardown) {
@@ -148,7 +135,6 @@ function exitProcess() {
 }
 
 function execMethod(method, args) {
-  // $FlowFixMe: This has to be a dynamic require.
   const main = require(file);
 
   let fn;

Test plan

Green CI

jeysal

Tried really hard to stay focused despite the huge diff, and found at least a few nits / questions 😅

jeysal · 2019-02-10T14:48:25Z

packages/jest-worker/src/types.ts

 // Because of the dynamic nature of a worker communication process, all messages
 // coming from any of the other processes cannot be typed. Thus, many types
-// include "any" as a flow type, which is (unfortunately) correct here.
+// include "unknown" as a TS type, which is (unfortunately) correct here.


Eagle eye 😄

jeysal · 2019-02-10T15:07:07Z

packages/jest-worker/src/types.ts

 };

 export type ChildMessageInitialize = [
  typeof CHILD_MESSAGE_INITIALIZE, // type
  boolean, // processed
  string, // file
-  ?Array<mixed>, // setupArgs
-  ?MessagePort, // MessagePort
+  Array<unknown> | undefined, // setupArgs


Intentionally left out null? I see ChildMessageInitialize being assigned from any and usually we have nulls in this package

Edit: I guess for an array leaving out the element makes more sense though. On a side note, wondering if TS differentiates between [string | undefined] and [string] | never[]. Would matter for inferring the parameter type of forEach 😄

yea, mostly because it didn't complain. the less allowed values the better, IMO. Easy enough to add more allowed types at some point if there's value in it

Tried out the side note thing in the TS playground btw, looks like it does differentiate 😎 not relevant for our use case though

packages/jest-worker/src/types.ts

jeysal · 2019-02-10T15:14:20Z

packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js

@@ -63,7 +61,7 @@ it('passes fork options down to child_process.fork, adding the defaults', () =>
  });

  expect(childProcess.fork.mock.calls[0][0]).toBe(child);
-  expect(childProcess.fork.mock.calls[0][1]).toEqual({
+  expect(childProcess.fork.mock.calls[0][2]).toEqual({


Wait what :D

See https://github.com/facebook/jest/pull/7853/files#diff-f9414f0ef9e6230a02ef6a47be731f55R55

The typings doesn't allow skipping the args array. It's not mentioned in the docs, so probably fine

Oh, totally missed the added [] ^^

Co-Authored-By: SimenB <sbekkhus91@gmail.com>

github-actions · 2021-05-12T01:30:15Z

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.

SimenB added this to the TypeScript Migration milestone Feb 10, 2019

facebook-github-bot added the cla signed label Feb 10, 2019

chore: migrate jest-worker to TypeScript

bf54943

SimenB force-pushed the worker branch from b08db21 to bf54943 Compare February 10, 2019 14:08

SimenB mentioned this pull request Feb 10, 2019

☂️TypeScript migration tracking issue #7807

Closed

jeysal suggested changes Feb 10, 2019

View reviewed changes

Update packages/jest-worker/src/types.ts

7cc3ce6

Co-Authored-By: SimenB <sbekkhus91@gmail.com>

SimenB merged commit b49075e into jestjs:master Feb 10, 2019

SimenB deleted the worker branch February 10, 2019 15:57

snyk-bot mentioned this pull request Dec 15, 2019

[Snyk] Upgrade babel-jest from 24.0.0 to 24.9.0 jakeherp/burger-builder#14

Open

This was referenced Mar 21, 2020

[Snyk] Upgrade: jest, jest-cli lwojcik/sc2pte-panel-frontend#88

Closed

[Snyk] Upgrade jest from 24.0.0 to 24.9.0 DavidKindler/burger-builder#47

Merged

github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: migrate jest-worker to TypeScript #7853

chore: migrate jest-worker to TypeScript #7853

SimenB commented Feb 10, 2019 •

edited

Loading

jeysal left a comment

jeysal Feb 10, 2019

jeysal Feb 10, 2019

SimenB Feb 10, 2019

jeysal Feb 10, 2019

jeysal Feb 10, 2019

SimenB Feb 10, 2019

jeysal Feb 10, 2019

github-actions bot commented May 12, 2021

chore: migrate jest-worker to TypeScript #7853

chore: migrate jest-worker to TypeScript #7853

Conversation

SimenB commented Feb 10, 2019 • edited Loading

Summary

Test plan

jeysal left a comment

Choose a reason for hiding this comment

jeysal Feb 10, 2019

Choose a reason for hiding this comment

jeysal Feb 10, 2019

Choose a reason for hiding this comment

SimenB Feb 10, 2019

Choose a reason for hiding this comment

jeysal Feb 10, 2019

Choose a reason for hiding this comment

jeysal Feb 10, 2019

Choose a reason for hiding this comment

SimenB Feb 10, 2019

Choose a reason for hiding this comment

jeysal Feb 10, 2019

Choose a reason for hiding this comment

github-actions bot commented May 12, 2021

SimenB commented Feb 10, 2019 •

edited

Loading