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

chore: migrate jest-serializer to TypeScript #7841

Merged
merged 5 commits into from Feb 9, 2019

Conversation

thymikee
Copy link
Collaborator

@thymikee thymikee commented Feb 8, 2019

Summary

Jumping on a TS train 馃殏

diff --git a/packages/jest-serializer/build/index.js b/packages/jest-serializer/build/index.js
index b6a60150d..1c0d8cdb7 100644
--- a/packages/jest-serializer/build/index.js
+++ b/packages/jest-serializer/build/index.js
@@ -3,8 +3,6 @@
  *
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
- *
- *
  */
 'use strict';

@@ -49,7 +47,7 @@ const JS_TYPE = '__$t__';
 const JS_VALUE = '__$v__';
 const JS_VF = '__$f__';

-function replacer(key, value) {
+function replacer(_key, value) {
   // NaN cannot be in a switch statement, because NaN !== NaN.
   if (Number.isNaN(value)) {
     return {
@@ -110,7 +108,7 @@ function replacer(key, value) {
   return value;
 }

-function reviver(key, value) {
+function reviver(_key, value) {
   if (!value || (typeof value !== 'object' && !value.hasOwnProperty(JS_TYPE))) {
     return value;
   }
@@ -157,15 +155,13 @@ function jsonStringify(content) {
   /* eslint-disable no-extend-native */

   try {
-    // $FlowFixMe: intentional removal of "toJSON" property.
-    Date.prototype.toJSON = undefined; // $FlowFixMe: intentional removal of "toJSON" property.
+    // @ts-ignore intentional removal of "toJSON" property.
+    Date.prototype.toJSON = undefined; // @ts-ignore intentional removal of "toJSON" property.

     Buffer.prototype.toJSON = undefined;
     return JSON.stringify(content, replacer);
   } finally {
-    // $FlowFixMe: intentional assignment of "toJSON" property.
-    Date.prototype.toJSON = dateToJSON; // $FlowFixMe: intentional assignment of "toJSON" property.
-
+    Date.prototype.toJSON = dateToJSON;
     Buffer.prototype.toJSON = bufferToJSON;
   }
   /* eslint-enable no-extend-native */
@@ -176,28 +172,24 @@ function jsonParse(content) {
 } // In memory functions.

 function deserialize(buffer) {
-  // $FlowFixMe - Node 8+ only
   return _v().default.deserialize
     ? _v().default.deserialize(buffer)
     : jsonParse(buffer.toString('utf8'));
 }

 function serialize(content) {
-  // $FlowFixMe - Node 8+ only
   return _v().default.serialize
     ? _v().default.serialize(content)
     : Buffer.from(jsonStringify(content));
 } // Synchronous filesystem functions.

 function readFileSync(filePath) {
-  // $FlowFixMe - Node 8+ only
   return _v().default.deserialize
     ? _v().default.deserialize(_fs().default.readFileSync(filePath))
     : jsonParse(_fs().default.readFileSync(filePath, 'utf8'));
 }

 function writeFileSync(filePath, content) {
-  // $FlowFixMe - Node 8+ only
   return _v().default.serialize
     ? _fs().default.writeFileSync(filePath, _v().default.serialize(content))
     : _fs().default.writeFileSync(filePath, jsonStringify(content), 'utf8');

Test plan

Green CI

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.

馃帀

@SimenB
Copy link
Member

SimenB commented Feb 8, 2019

Changelog?

@thymikee
Copy link
Collaborator Author

thymikee commented Feb 8, 2019

Good call!

@@ -18,7 +18,9 @@ import serializer from '..';

const v8s = [
{
// @ts-ignore - Node 8+ only
Copy link

Choose a reason for hiding this comment

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

Instead of disabling all those node 8 checks, you should mybe patch the node types.

declare namespace NodeJS {
  interface v8 {
    // ...
  }
}

Copy link
Collaborator Author

@thymikee thymikee Feb 8, 2019

Choose a reason for hiding this comment

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

Thanks! Added global.d.ts, hope it's ok? I'm new to TS :P

@thymikee thymikee force-pushed the chore/ts-serializer branch 5 times, most recently from 37fabc8 to 6a2b5ca Compare February 8, 2019 21:56
@codecov-io
Copy link

codecov-io commented Feb 8, 2019

Codecov Report

Merging #7841 into master will decrease coverage by 0.17%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7841      +/-   ##
==========================================
- Coverage    64.8%   64.63%   -0.18%     
==========================================
  Files         230      229       -1     
  Lines        8657     8615      -42     
  Branches        5        5              
==========================================
- Hits         5610     5568      -42     
  Misses       3045     3045              
  Partials        2        2

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 d96ff2c...3123acf. Read the comment docs.

SimenB and others added 2 commits February 9, 2019 10:16
Co-Authored-By: thymikee <thymikee@gmail.com>
@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

5 participants