chore: replace `any`s with `unknown`s #9626

G-Rath · 2020-02-29T06:55:02Z

Summary

This replaces some of the any types in the codebase with either unknown or a more accurate type.
Also removes some casts in favor of using the generic parameter when calling `reduce.

Test plan

Make a PR, and see if it blows up :D

G-Rath · 2020-02-29T06:56:10Z

packages/jest-docblock/src/index.ts

@@ -18,6 +18,7 @@ const ltrimNewlineRe = /^(\r?\n)+/;
 const multilineRe = /(?:^|\r?\n) *(@[^\r\n]*?) *\r?\n *(?![^@\r\n]*\/\/[^]*)([^@\r\n\s][^@\r\n]+?) *\r?\n/g;
 const propertyRe = /(?:^|\r?\n) *@(\S+) *([^\r\n]*)/g;
 const stringStartRe = /(\r?\n|^) *\* ?/g;
+const STRING_ARRAY: Array<string> = [];


This in theory is more performant as it's not allocating a new array every time, but mainly it just lets us drop as Array<string> in two places

G-Rath · 2020-02-29T06:57:16Z

packages/pretty-format/src/plugins/DOMCollection.ts

-          name === 'NamedNodeMap'
-            ? Array.prototype.reduce.call(collection, propsReducer, {})
+          isNamedNodeMap(collection)
+            ? Array.from(collection).reduce<Record<string, string>>(


I inlined this function as there's no way to pass a generic parameter otherwise; it's inlined in DOMElement, and so now actually matches 1:1.

G-Rath · 2020-02-29T06:57:55Z

packages/pretty-format/src/plugins/DOMElement.ts

-            props[attribute.name] = attribute.value;
-            return props;
-          }, {} as any),
+        ? {}


This could actually be classed as a bug - it might not ever be reachable due to surrounding code, but still - it was an array when it should have been an object 🤷‍♂

codecov-io · 2020-02-29T07:11:57Z

Codecov Report

Merging #9626 into master will increase coverage by 0.00%.
The diff coverage is 79.31%.

@@           Coverage Diff           @@
##           master    #9626   +/-   ##
=======================================
  Coverage   64.90%   64.90%           
=======================================
  Files         288      288           
  Lines       12195    12197    +2     
  Branches     3022     3022           
=======================================
+ Hits         7915     7917    +2     
  Misses       3639     3639           
  Partials      641      641

Impacted Files	Coverage Δ
packages/expect/src/matchers.ts	`96.86% <ø> (ø)`
packages/expect/src/toThrowMatchers.ts	`86.48% <ø> (ø)`
...ckages/jest-config/src/ReporterValidationErrors.ts	`11.76% <ø> (ø)`
packages/jest-config/src/normalize.ts	`76.66% <ø> (ø)`
packages/jest-core/src/FailedTestsCache.ts	`76.47% <ø> (ø)`
packages/jest-fake-timers/src/jestFakeTimers.ts	`90.35% <ø> (ø)`
packages/jest-jasmine2/src/jasmine/Env.ts	`0.00% <ø> (ø)`
packages/jest-jasmine2/src/jasmine/spyRegistry.ts	`0.00% <ø> (ø)`
packages/jest-resolve/src/nodeModulesPaths.ts	`85.71% <ø> (ø)`
packages/pretty-format/src/collections.ts	`98.48% <ø> (ø)`
... and 10 more

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 26951bd...4118c8d. Read the comment docs.

G-Rath · 2020-02-29T23:34:17Z

ci/circleci: test-browser:

error https://registry.yarnpkg.com/path-exists/-/path-exists-3.0.0.tgz: Extracting tar content of undefined failed, the file appears to be corrupt: "ENOSPC: no space left on device, write"

😬

G-Rath · 2020-03-13T23:19:56Z

Failing test seems to be unrelated - it's failing on a different worker related test on each action

SimenB · 2020-03-29T15:53:49Z

packages/expect/src/matchers.ts

-  toBeInstanceOf(this: MatcherState, received: any, expected: Function) {
+  toBeInstanceOf(
+    this: MatcherState,
+    received: new () => unknown,


I don't think these are safe, I'd prefer to use unknown and narrow down. We're supposed to throw if they are called with the wrong type anyways, and making those checks narrow the type seems ideal. These get called by user code who provides whatever they feel like.

Yeah you're totally right!

SimenB · 2020-03-29T15:55:02Z

packages/jest-jasmine2/src/jasmine/spyRegistry.ts

@@ -39,7 +39,7 @@ const formatErrorMsg = (domain: string, usage?: string) => {
  return (msg: string) => domain + ' : ' + msg + usageDefinition;
 };

-function isSpy(putativeSpy: any) {
+function isSpy(putativeSpy: Spy) {


this is supposed to be a typeguard, so maybe

Suggested change

function isSpy(putativeSpy: Spy) {

function isSpy(putativeSpy: unknown): putativeSpy is Spy {

?

SimenB

Thanks! Sorry about the slow response, been swamped lately.

I think most of these changes are great, but the changes in expect are not necessarily right I think (although they're not less right than any, but any is more honest)

SimenB

thanks!

…pshots * upstream/master: (225 commits) docs: add CLA link to contributing docs (jestjs#9789) chore: roll new version of docs v25.3.0 chore: update changelog for release chore(jest-types): correct type testRegex for ProjectConfig (jestjs#9780) feat(circus): enable writing async test event handlers (jestjs#9397) feat: enable all babel syntax plugins (jestjs#9774) chore: add helper for getting Jest's config in e2e tests (jestjs#9770) feat: pass ESM options to transformers (jestjs#9597) chore: replace `any`s with `unknown`s (jestjs#9626) feat: pass ESM options to Babel (jestjs#9766) chore(website): add copy button the code blocks (jestjs#9750) chore: bump istanbul-reports for new uncovered lines design (jestjs#9758) chore: correct CHANGELOG.md (jestjs#9763) chore(jest-types): expose type `CacheKeyOptions` for `getCacheK… (jestjs#9762) docs: Fix simple typo, seperated -> separated (jestjs#9760) v25.2.7 chore: update changelog for release fix: drop getters and setters when diffing objects for error (jestjs#9757) chore(jest-types): correct return type of shouldRunTestSuite fo… (jestjs#9753) ...

sandersn · 2020-04-15T22:58:20Z

Note: this breaks a couple of tests in @types/jest's test suite:

expect.addSnapshotSerializer({
    print: (value: {}) => '',
    test: (value: {}) => value === value,
});

expect.addSnapshotSerializer({
    print: (value: {}, serialize: (val: {}) => string, indent: (str: string) => string, opts: {}) => '',
    test: (value: {}) => value === value,
});

unknown is not assignable to {}, so the first value now has to have the type unknown instead:

expect.addSnapshotSerializer({
    print: (value: unknown) => '',
    test: (value: {}) => value === value,
});

expect.addSnapshotSerializer({
    print: (value: unknown, serialize: (val: {}) => string, indent: (str: string) => string, opts: {}) => '',
    test: (value: {}) => value === value,
});

No idea if this example is realistic or not, I just noticed it while running the TS team's Definitely Typed tests. It seems reasonable that a serialiser should handle any JS value, including null and undefined.

SimenB · 2020-04-16T06:53:18Z

I think it makes sense. Ideally test would work as a typeguard for print, but I'm not sure how to achieve that?

G-Rath · 2020-04-16T07:18:10Z

I think it makes sense.

if it doesn't then the type should be object or narrower ;)

Ideally test would work as a typeguard for print

If I understand correctly:

type Serializer<T> = {
  print: (val: T, serialize: JestSerialize, indent: number) => string;
  test: (val: unknown) => val is T;
};

I can't find the actual places in code where this is used, but that should do the trick if you're just doing:

if(serializer.test(val)) {
  serializer.print(val, /* ... everything else ... */);
}

G-Rath · 2020-04-16T07:22:45Z

Oh also rereading @sandersn's example I think while yes the type is more accurate now, the value parameter should have the same type for both functions

i.e

// incorrect
expect.addSnapshotSerializer({
    print: (value: unknown) => '',
    test: (value: {}) => value === value,
});

// accurate
expect.addSnapshotSerializer({
    print: (value: unknown) => '',
    test: (value: unknown) => value === value,
});

// spot on
expect.addSnapshotSerializer({
    print: (value: MyType) => '',
    test: (value: unknown): value is MyType => value === value,
});

github-actions · 2021-05-11T09:06:51Z

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.

facebook-github-bot added the cla signed label Feb 29, 2020

G-Rath commented Feb 29, 2020

View reviewed changes

SimenB reviewed Mar 29, 2020

View reviewed changes

SimenB requested changes Mar 29, 2020

View reviewed changes

G-Rath added 7 commits April 3, 2020 12:34

chore(pretty-format): replace usages of any with stricter types

a7df909

chore: replace any types with stronger types or unknown

cc1eb06

chore: use generic parameter when calling reduce instead of casting

4e5cbfd

chore(jest-core): use explict type instead of as cast

3a374dc

chore(jest-docblock): mark array as readonly

4b6e550

chore(jest-jasmine2): make isSpy function a typeguard

bb3dc5c

chore(expect): use any type for actual parameter

4118c8d

G-Rath requested a review from SimenB April 5, 2020 01:47

SimenB approved these changes Apr 5, 2020

View reviewed changes

SimenB added 2 commits April 5, 2020 10:45

Merge branch 'master' into replace-anys-with-unknowns

26ec098

changelog

fcca028

SimenB changed the title ~~Replace anys with unknowns~~ chore: replace anys with unknowns Apr 5, 2020

SimenB merged commit 8b8f90a into jestjs:master Apr 5, 2020

nfantone mentioned this pull request Apr 10, 2020

Testing arity of functions no longer works #9792

Closed

G-Rath deleted the replace-anys-with-unknowns branch April 16, 2020 00:36

This was referenced May 1, 2020

[Snyk] Upgrade jest from 25.2.7 to 25.3.0 mkai/express-absolute-url#10

Merged

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

Closed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: replace `any`s with `unknown`s #9626

chore: replace `any`s with `unknown`s #9626

G-Rath commented Feb 29, 2020

G-Rath Feb 29, 2020 •

edited

Loading

G-Rath Feb 29, 2020

G-Rath Feb 29, 2020

codecov-io commented Feb 29, 2020 •

edited

Loading

G-Rath commented Feb 29, 2020

G-Rath commented Mar 13, 2020

SimenB Mar 29, 2020

G-Rath Apr 2, 2020

SimenB Mar 29, 2020

SimenB left a comment

SimenB left a comment

sandersn commented Apr 15, 2020

SimenB commented Apr 16, 2020

G-Rath commented Apr 16, 2020

G-Rath commented Apr 16, 2020 •

edited

Loading

github-actions bot commented May 11, 2021

	function isSpy(putativeSpy: Spy) {
	function isSpy(putativeSpy: unknown): putativeSpy is Spy {

chore: replace anys with unknowns #9626

chore: replace anys with unknowns #9626

Conversation

G-Rath commented Feb 29, 2020

Summary

Test plan

G-Rath Feb 29, 2020 • edited Loading

Choose a reason for hiding this comment

G-Rath Feb 29, 2020

Choose a reason for hiding this comment

G-Rath Feb 29, 2020

Choose a reason for hiding this comment

codecov-io commented Feb 29, 2020 • edited Loading

Codecov Report

G-Rath commented Feb 29, 2020

G-Rath commented Mar 13, 2020

SimenB Mar 29, 2020

Choose a reason for hiding this comment

G-Rath Apr 2, 2020

Choose a reason for hiding this comment

SimenB Mar 29, 2020

Choose a reason for hiding this comment

SimenB left a comment

Choose a reason for hiding this comment

SimenB left a comment

Choose a reason for hiding this comment

sandersn commented Apr 15, 2020

SimenB commented Apr 16, 2020

G-Rath commented Apr 16, 2020

G-Rath commented Apr 16, 2020 • edited Loading

github-actions bot commented May 11, 2021

chore: replace `any`s with `unknown`s #9626

chore: replace `any`s with `unknown`s #9626

G-Rath Feb 29, 2020 •

edited

Loading

codecov-io commented Feb 29, 2020 •

edited

Loading

G-Rath commented Apr 16, 2020 •

edited

Loading