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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate jest-leak-detector to TypeScript #7825

Merged
merged 9 commits into from Feb 7, 2019

Conversation

r3nya
Copy link
Contributor

@r3nya r3nya commented Feb 7, 2019

Summary

Hello! It my first PR, if I do something wrong, please let me know. 馃槈

Diff
diff --git a/packages/jest-leak-detector/build/index.js b/packages/jest-leak-detector/build/index.js
index 5402915b8..ff851b3fb 100644
--- a/packages/jest-leak-detector/build/index.js
+++ b/packages/jest-leak-detector/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', {
@@ -13,30 +5,30 @@ Object.defineProperty(exports, '__esModule', {
 });
 exports.default = void 0;
 
-function _prettyFormat() {
-  const data = _interopRequireDefault(require('pretty-format'));
+function _v() {
+  const data = _interopRequireDefault(require('v8'));
 
-  _prettyFormat = function _prettyFormat() {
+  _v = function _v() {
     return data;
   };
 
   return data;
 }
 
-function _v() {
-  const data = _interopRequireDefault(require('v8'));
+function _vm() {
+  const data = _interopRequireDefault(require('vm'));
 
-  _v = function _v() {
+  _vm = function _vm() {
     return data;
   };
 
   return data;
 }
 
-function _vm() {
-  const data = _interopRequireDefault(require('vm'));
+function _prettyFormat() {
+  const data = _interopRequireDefault(require('pretty-format'));
 
-  _vm = function _vm() {
+  _prettyFormat = function _prettyFormat() {
     return data;
   };
 
@@ -47,6 +39,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.
+ */
 class _default {
   constructor(value) {
     if (this._isPrimitive(value)) {
diff --git a/packages/jest-leak-detector/build/index.ts_ b/packages/jest-leak-detector/build/index.ts_
new file mode 100644
index 000000000..c5ad07ba7
--- /dev/null
+++ b/packages/jest-leak-detector/build/index.ts_
@@ -0,0 +1,72 @@
+/**
+ * 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';
+
+import prettyFormat from 'pretty-format';
+import v8 from 'v8';
+import vm from 'vm';
+
+export default class {
+  _isReferenceBeingHeld: boolean;
+
+  constructor(value: ?Object) {
+    if (this._isPrimitive(value)) {
+      throw new TypeError(
+        [
+          'Primitives cannot leak memory.',
+          'You passed a ' + typeof value + ': <' + prettyFormat(value) + '>',
+        ].join(' '),
+      );
+    }
+
+    let weak;
+
+    try {
+      // eslint-disable-next-line import/no-extraneous-dependencies
+      weak = require('weak');
+    } catch (err) {
+      if (!err || err.code !== 'MODULE_NOT_FOUND') {
+        throw err;
+      }
+
+      throw new Error(
+        'The leaking detection mechanism requires the "weak" package to be installed and work. ' +
+          'Please install it as a dependency on your main project',
+      );
+    }
+
+    weak(value, () => (this._isReferenceBeingHeld = false));
+    this._isReferenceBeingHeld = true;
+
+    // Ensure value is not leaked by the closure created by the "weak" callback.
+    value = null;
+  }
+
+  isLeaking(): boolean {
+    this._runGarbageCollector();
+
+    return this._isReferenceBeingHeld;
+  }
+
+  _runGarbageCollector() {
+    const isGarbageCollectorHidden = !global.gc;
+
+    // GC is usually hidden, so we have to expose it before running.
+    v8.setFlagsFromString('--expose-gc');
+    vm.runInNewContext('gc')();
+
+    // The GC was not initially exposed, so let's hide it again.
+    if (isGarbageCollectorHidden) {
+      v8.setFlagsFromString('--no-expose-gc');
+    }
+  }
+
+  _isPrimitive(value: any): boolean {
+    return value !== Object(value);
+  }
+}


Test plan

馃挌 build;

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@SimenB SimenB added this to the TypeScript Migration milestone Feb 7, 2019
@SimenB
Copy link
Member

SimenB commented Feb 7, 2019

Thanks!

Couple of things:

  • The diff should be against the build on master. The point is to inspect the difference in the transpiled code so we can be sure there's no breaking change
  • Don't push the commit with build artifacts - it's just for inspection before we merge 馃檪

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@r3nya r3nya force-pushed the chore/migrate-jest-leak-detector-to-ts branch from 49674c2 to a00add6 Compare February 7, 2019 10:08
@r3nya r3nya force-pushed the chore/migrate-jest-leak-detector-to-ts branch from 309e4f2 to 55ef0f7 Compare February 7, 2019 10:45
packages/jest-leak-detector/src/index.ts Outdated Show resolved Hide resolved
packages/jest-leak-detector/src/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

If CI is happy, this should be good to go 馃檪

@r3nya
Copy link
Contributor Author

r3nya commented Feb 7, 2019

@SimenB @thymikee thanks a lot for your help guys! 馃挌 CI now.

@SimenB SimenB merged commit 5c18df6 into jestjs:master Feb 7, 2019
@r3nya r3nya deleted the chore/migrate-jest-leak-detector-to-ts branch February 7, 2019 11:14
captain-yossarian pushed a commit to captain-yossarian/jest that referenced this pull request Jul 18, 2019
@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.

None yet

4 participants