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

jest snapshot - TS migration #7899

Merged
merged 37 commits into from Feb 16, 2019
Merged

jest snapshot - TS migration #7899

merged 37 commits into from Feb 16, 2019

Conversation

doniyor2109
Copy link
Contributor

@doniyor2109 doniyor2109 commented Feb 14, 2019

Summary

TS migration for jest-snapshot

Built diff:

diff --git c/packages/jest-snapshot/build/State.js w/packages/jest-snapshot/build/State.js
index 4c61197da..b50ddaea7 100644
--- c/packages/jest-snapshot/build/State.js
+++ w/packages/jest-snapshot/build/State.js
@@ -24,6 +24,7 @@ var jestExistsFile =
   global[Symbol.for('jest-native-exists-file')] || _fs.default.existsSync;
 
 class SnapshotState {
+  // @ts-ignore
   constructor(snapshotPath, options) {
     this._snapshotPath = snapshotPath;
 
@@ -63,7 +64,7 @@ class SnapshotState {
 
     if (options.isInline) {
       const error = options.error || new Error();
-      const lines = (0, _jestMessageUtil.getStackTraceLines)(error.stack);
+      const lines = (0, _jestMessageUtil.getStackTraceLines)(error.stack || '');
       const frame = (0, _jestMessageUtil.getTopFrame)(lines);
 
       if (!frame) {
@@ -238,7 +239,7 @@ class SnapshotState {
     }
   }
 
-  fail(testName, received, key) {
+  fail(testName, _, key) {
     this._counters.set(testName, (this._counters.get(testName) || 0) + 1);
 
     const count = Number(this._counters.get(testName));
diff --git c/packages/jest-snapshot/build/inline_snapshots.js w/packages/jest-snapshot/build/inline_snapshots.js
index d813312b9..929212b7c 100644
--- c/packages/jest-snapshot/build/inline_snapshots.js
+++ w/packages/jest-snapshot/build/inline_snapshots.js
@@ -7,10 +7,10 @@ exports.saveInlineSnapshots = void 0;
 
 var _fs = _interopRequireDefault(require('fs'));
 
-var _semver = _interopRequireDefault(require('semver'));
-
 var _path = _interopRequireDefault(require('path'));
 
+var _semver = _interopRequireDefault(require('semver'));
+
 var _types = require('@babel/types');
 
 var _utils = require('./utils');
@@ -135,8 +135,10 @@ const groupSnapshotsBy = createKey => snapshots =>
     });
   }, {});
 
-const groupSnapshotsByFrame = groupSnapshotsBy(
-  ({frame: {line, column}}) => `${line}:${column - 1}`
+const groupSnapshotsByFrame = groupSnapshotsBy(({frame: {line, column}}) =>
+  typeof line === 'number' && typeof column === 'number'
+    ? `${line}:${column - 1}`
+    : ''
 );
 const groupSnapshotsByFile = groupSnapshotsBy(({frame: {file}}) => file);
 
diff --git c/packages/jest-snapshot/build/mock_serializer.js w/packages/jest-snapshot/build/mock_serializer.js
index 4965cafc1..b7a339c62 100644
--- c/packages/jest-snapshot/build/mock_serializer.js
+++ w/packages/jest-snapshot/build/mock_serializer.js
@@ -10,8 +10,6 @@ exports.default = exports.test = exports.serialize = 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 serialize = (val, config, indentation, depth, refs, printer) => {
   // Serialize a non-default name, even if config.printFunctionName is false.
@@ -46,8 +44,9 @@ exports.serialize = serialize;
 const test = val => val && !!val._isMockFunction;
 
 exports.test = test;
-var _default = {
+const plugin = {
   serialize,
   test
 };
+var _default = plugin;
 exports.default = _default;
diff --git c/packages/jest-snapshot/build/plugins.js w/packages/jest-snapshot/build/plugins.js
index 26f3c1644..1cbaf464a 100644
--- c/packages/jest-snapshot/build/plugins.js
+++ w/packages/jest-snapshot/build/plugins.js
@@ -18,8 +18,6 @@ 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 _prettyFormat$plugins = _prettyFormat.default.plugins,
   DOMCollection = _prettyFormat$plugins.DOMCollection,
diff --git c/packages/jest-snapshot/build/snapshot_resolver.js w/packages/jest-snapshot/build/snapshot_resolver.js
index 868ee8ab0..13150b9a8 100644
--- c/packages/jest-snapshot/build/snapshot_resolver.js
+++ w/packages/jest-snapshot/build/snapshot_resolver.js
@@ -5,15 +5,20 @@ Object.defineProperty(exports, '__esModule', {
 });
 exports.buildSnapshotResolver = exports.isSnapshotPath = exports.DOT_EXTENSION = exports.EXTENSION = void 0;
 
-var _chalk = _interopRequireDefault(require('chalk'));
-
 var _path = _interopRequireDefault(require('path'));
 
+var _chalk = _interopRequireDefault(require('chalk'));
+
 function _interopRequireDefault(obj) {
   return obj && obj.__esModule ? obj : {default: obj};
 }
 
-// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+/**
+ * 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 EXTENSION = 'snap';
 exports.EXTENSION = EXTENSION;
 const DOT_EXTENSION = '.' + EXTENSION;
@@ -66,11 +71,12 @@ function createDefaultSnapshotResolver() {
 function createCustomSnapshotResolver(snapshotResolverPath) {
   const custom = require(snapshotResolverPath);
 
-  [
+  const keys = [
     ['resolveSnapshotPath', 'function'],
     ['resolveTestPath', 'function'],
     ['testPathForConsistencyCheck', 'string']
-  ].forEach(([propName, requiredType]) => {
+  ];
+  keys.forEach(([propName, requiredType]) => {
     if (typeof custom[propName] !== requiredType) {
       throw new TypeError(mustImplement(propName, requiredType));
     }
diff --git c/packages/jest-snapshot/build/types.js w/packages/jest-snapshot/build/types.js
new file mode 100644
index 000000000..ad9a93a7c
--- /dev/null
+++ w/packages/jest-snapshot/build/types.js
@@ -0,0 +1 @@
+'use strict';
diff --git c/packages/jest-snapshot/build/utils.js w/packages/jest-snapshot/build/utils.js
index 552da01d1..1b1aa3932 100644
--- c/packages/jest-snapshot/build/utils.js
+++ w/packages/jest-snapshot/build/utils.js
@@ -5,20 +5,20 @@ Object.defineProperty(exports, '__esModule', {
 });
 exports.deepMerge = exports.saveSnapshotFile = exports.ensureDirectoryExists = exports.escapeBacktickString = exports.unescape = exports.serialize = exports.getSnapshotData = exports.keyToTestName = exports.testNameToKey = exports.SNAPSHOT_VERSION_WARNING = exports.SNAPSHOT_GUIDE_LINK = exports.SNAPSHOT_VERSION = void 0;
 
-var _plugins = require('./plugins');
-
-var _chalk = _interopRequireDefault(require('chalk'));
-
 var _fs = _interopRequireDefault(require('fs'));
 
+var _path = _interopRequireDefault(require('path'));
+
 var _mkdirp = _interopRequireDefault(require('mkdirp'));
 
 var _naturalCompare = _interopRequireDefault(require('natural-compare'));
 
-var _path = _interopRequireDefault(require('path'));
+var _chalk = _interopRequireDefault(require('chalk'));
 
 var _prettyFormat = _interopRequireDefault(require('pretty-format'));
 
+var _plugins = require('./plugins');
+
 function _interopRequireDefault(obj) {
   return obj && obj.__esModule ? obj : {default: obj};
 }
@@ -169,8 +169,7 @@ const getSnapshotData = (snapshotPath, update) => {
     try {
       snapshotContents = jestReadFile(snapshotPath, 'utf8'); // eslint-disable-next-line no-new-func
 
-      const populate = new Function('exports', snapshotContents); // $FlowFixMe
-
+      const populate = new Function('exports', snapshotContents);
       populate(data);
     } catch (e) {}
   }

Test plan

yarn test

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.

Thanks for the PR, this is great! Left som inline comments.

The big one is that it seems we have to stick some more types into @jest/types in order to avoid changing the exported interface of the modules we have that are stuck on CJS

packages/jest-snapshot/dts/natural-compare.d.ts Outdated Show resolved Hide resolved
packages/jest-snapshot/src/State.ts Show resolved Hide resolved
packages/jest-snapshot/src/__tests__/throw_matcher.test.ts Outdated Show resolved Hide resolved
import diff from 'jest-diff';
import {EXPECTED_COLOR, matcherHint, RECEIVED_COLOR} from 'jest-matcher-utils';
import {MatcherState} from 'expect';
Copy link
Member

Choose a reason for hiding this comment

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

an you put this in @jest/types instead? We'll move it back into expect when it is migrated to TS (or probably keep it in @jest/types until the next major since expect doesn't use ESM)

Copy link
Member

Choose a reason for hiding this comment

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

/cc @natealcedo relevant for your expect migration 🙂

packages/jest-snapshot/src/utils.ts Outdated Show resolved Hide resolved
packages/jest-snapshot/src/index.ts Outdated Show resolved Hide resolved
packages/jest-snapshot/src/index.ts Show resolved Hide resolved
packages/jest-snapshot/src/mock_serializer.ts Outdated Show resolved Hide resolved
packages/jest-snapshot/tsconfig.json Outdated Show resolved Hide resolved
packages/jest-snapshot/package.json Outdated Show resolved Hide resolved
doniyor2109 and others added 2 commits February 15, 2019 17:41
Co-Authored-By: doniyor2109 <doniyor2109@gmail.com>
doniyor2109 and others added 2 commits February 15, 2019 17:47
Co-Authored-By: doniyor2109 <doniyor2109@gmail.com>
Copy link
Contributor Author

@doniyor2109 doniyor2109 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 15, 2019

@doniyor2109 all module.exports = must be made into export = so that TS is able to generate the correct d.ts files for them

@doniyor2109
Copy link
Contributor Author

@SimenB That is ok. Thank you for great feedbacks. Just keep going giving great feedbacks 😄

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.

Getting closer! 😀

packages/jest-snapshot/dts/natural-compare.d.ts Outdated Show resolved Hide resolved
packages/jest-snapshot/src/__tests__/matcher.test.ts Outdated Show resolved Hide resolved
packages/jest-snapshot/src/__tests__/matcher.test.ts Outdated Show resolved Hide resolved
packages/jest-snapshot/src/index.ts Outdated Show resolved Hide resolved
packages/jest-snapshot/src/plugins.ts Outdated Show resolved Hide resolved
packages/jest-snapshot/src/snapshot_resolver.ts Outdated Show resolved Hide resolved
packages/jest-snapshot/src/types.ts Outdated Show resolved Hide resolved
SimenB and others added 3 commits February 15, 2019 21:34
Co-Authored-By: doniyor2109 <doniyor2109@gmail.com>
@SimenB
Copy link
Member

SimenB commented Feb 16, 2019

I merged in master and fixed the type error, so now we should be able to see if we've broken any tests 🙂

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7899      +/-   ##
==========================================
- Coverage   57.98%   57.91%   -0.07%     
==========================================
  Files         171      164       -7     
  Lines        6422     6093     -329     
  Branches        6        5       -1     
==========================================
- Hits         3724     3529     -195     
+ Misses       2696     2562     -134     
  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 d9d501a...19aa286. Read the comment docs.

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.

Thank you so much! Took a bit to get it over the line, but this is really awesome work

packages/pretty-format/tsconfig.json Outdated Show resolved Hide resolved
@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