Chore(eslint):- Turned `no-implicit-coercion` rule to error and fixed all the occurrences #12476

Biki-das · 2022-02-23T15:23:55Z

first of all, sorry @SimenB, I messed up my last PR and it's my fault and I know how much annoying this becomes, I will try my best not to repeat this mistake

this PR is extended of #12457, and the rule of no-implicit-coercion was already added , i turned it to error and fixed the occurrences

SimenB · 2022-02-23T15:46:56Z

first of all, sorry @SimenB, I messed up my last PR and it's my fault and I know how much annoying this becomes, I will try my best not to repeat this mistake

It's not a huge deal, don't sweat it! 😀

Ah, you found a lint rule! Sweet. It seems there are still errors reported by it 🙂

Smrtnyk · 2022-02-23T16:00:38Z

packages/jest-serializer/src/__tests__/index.test.ts

@@ -16,7 +16,7 @@ const objs = [
  null,
  [0, true, '2', [3.14, {}, null]],
  {key1: 'foo', key2: 'bar', key3: {array: [null, {}]}},
-  {minusInf: -Infinity, nan: NaN, plusInf: +Infinity},
+  {minusInf: Number(Infinity), nan: NaN, plusInf: +Infinity},


negative infinity is a valid thing
you could put here Number.NEGATIVE_INFINITY

and Number.POSITIVE_INFINITY for plusInf

Smrtnyk · 2022-02-23T16:06:18Z

packages/jest-transform/src/shouldInstrument.ts

@@ -81,7 +81,9 @@ export default function shouldInstrument(
  }

  if (
-    config.coveragePathIgnorePatterns.some(pattern => !!filename.match(pattern))
+    config.coveragePathIgnorePatterns.some(pattern =>


Is something putting stuff onto the next line maybe?
these changes do not seem related to the rule itself
I don't mind it, but am wondering if you are using some autofix for this

Smrtnyk · 2022-02-23T16:06:53Z

packages/jest-util/src/createProcessObject.ts

@@ -64,7 +64,7 @@ function createProcessEnv(): NodeJS.ProcessEnv {
    get: isWin32 ? getPropertyWin32 : getProperty,

    set(_target, key, value) {
-      const strValue = '' + value;
+      const strValue = '' + String(value);


this seems weird
'' + will end up as string anyway
maybe just leave String(value)

Smrtnyk · 2022-02-23T16:08:36Z

packages/pretty-format/src/index.ts

@@ -128,7 +128,7 @@ function printBasicValue(
  escapeString: boolean,
 ): string | null {
  if (val === true || val === false) {
-    return '' + val;
+    return '' + String(val);


Same here, I think you should drop the '' + part

Biki-das · 2022-02-23T16:26:57Z

first of all, sorry @SimenB, I messed up my last PR and it's my fault and I know how much annoying this becomes, I will try my best not to repeat this mistake

It's not a huge deal, don't sweat it! 😀

Ah, you found a lint rule! Sweet. It seems there are still errors reported by it 🙂

Yeah i am working on it

SimenB · 2022-02-23T23:43:07Z

Lots of these !!thing -> Boolean(thing) changes should instead be thing != null

Biki-das · 2022-02-24T05:22:47Z

Lots of these !!thing -> Boolean(thing) changes should instead be thing != null

thanks for that

Biki-das · 2022-02-24T07:18:24Z

@SimenB getting partial errors with Ci anything you can suggest?😐😐

SimenB · 2022-02-24T08:03:59Z

It means some behaviour changed I guess

Biki-das · 2022-02-26T14:55:30Z

@SimenB this is good to go , finally was able to made it work :-)

Smrtnyk · 2022-02-26T15:23:14Z

packages/expect-utils/src/utils.ts

@@ -137,7 +137,7 @@ export const getObjectSubset = (
 const IteratorSymbol = Symbol.iterator;

 const hasIterator = (object: any) =>
-  !!(object != null && object[IteratorSymbol]);
+  Boolean(object != null && object[IteratorSymbol]);


object != null already yields boolean, so it is wrong
you want something like object != null && Boolean(object[IteratorSymbol]);

I think we should do object != null && object[IteratorSymbol] != null

yeah i am sort of confused but this looks good

Smrtnyk · 2022-02-26T15:24:27Z

packages/expect/src/__tests__/extend.test.ts

@@ -109,7 +109,7 @@ it('exposes an equality function to custom matchers', () => {
  jestExpect.extend({
    toBeOne() {
      expect(this.equals).toBe(equals);
-      return {pass: !!this.equals(1, 1)};
+      return {pass: Boolean(this.equals(1, 1))};


I would expect from something that is named like equals to already return boolean :)

then what should i do here?

that was just a comment that naming of the function is ambiguous, but if it does not return a boolean you did then what you had to

this.equals does return a boolean, so just remove the !!

Smrtnyk · 2022-02-26T15:26:42Z

packages/jest-core/src/lib/activeFiltersMessage.ts

@@ -27,7 +27,7 @@ const activeFilters = (

    const messages = ['\n' + chalk.bold('Active Filters: ') + filters];

-    return messages.filter(message => !!message).join(delimiter);
+    return messages.filter(message => Boolean(message)).join(delimiter);


you could have this shorter and receiveBoolean wrapper as a reference
something like
return messages.filter(Boolean).join(delimiter);
but I guess ts will complain because signatures do not match?

yes i tried to do that 😀

I think a return messages.filter(message => message.length > 0).join(delimiter); is better

Smrtnyk · 2022-02-26T15:29:12Z

packages/jest-docblock/src/__tests__/index.test.ts

-      EOL +
-      '' +
-      ' */';
+      String(


this looks like a pure mess :)

I think what you need here is to just wrap EOL in String wrapper
like String(EOL) and leave the rest as is

ok sure 😀

Smrtnyk · 2022-02-26T15:30:06Z

packages/jest-docblock/src/__tests__/index.test.ts

-      '' +
-      ' */';
+      String(
+        String(String('/**' + EOL) + ' * @team foo' + EOL) +


same here, from readability perspective this is not good
try to wrap just EOL instances like String(EOL)

Smrtnyk · 2022-02-26T15:36:28Z

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

@@ -98,10 +98,10 @@ export default class Spec {
  static pendingSpecExceptionMessage: string;

  static isPendingSpecException(e: Error) {
-    return !!(
+    return Boolean(


there is one eslint rule which imho would help with code blocks with lots of operators
https://eslint.org/docs/rules/operator-linebreak
so it would look like

e && e.toString && e.toString()...

having stuff at the start of the line is usually easier to keep cognitive complexity on the brain in place.
You can negotiate it with Simen if he wants it, it would be a good issue if you want to tackle it

yeah sure let me just clear this one and then discuss with simen 😀

e.toString?.().includes(Spec.pendingSpecExceptionMessage) or something probably, but just disable the rule in this (and other packages/jest-jasmine2/src/jasmine/*.ts files) - they're forked from jasmine and can remain a bit messy 🙂

@SimenB disable no-implicit-coercion rule?

yes, in this and the other jasmine files

packages/jest-mock/src/index.ts

SimenB · 2022-02-26T16:04:22Z

I don't like Boolean even if the lint rule suggests it 🙂 almost every place it should be != null or if it's a string, we should check the strings length

Smrtnyk · 2022-02-26T16:06:41Z

I don't like Boolean even if the lint rule suggests it slightly_smiling_face almost every place it should be != null or if it's a string, we should check the strings length

I personally find this rule more noisy than useful :)

SimenB · 2022-02-26T16:24:57Z

I'm a fan of getting rid of double bangs (they're quite unreadable and very JS specific) in favour of more explicit checks

Biki-das · 2022-02-26T17:04:57Z

@SimenB done :-) with the suggested changes

SimenB · 2022-02-26T23:29:13Z

e2e/setup-files-after-env-config/setupHooksIntoRunner.js

@@ -5,4 +5,4 @@
 * LICENSE file in the root directory of this source tree.
 */

-globalThis.describeDefined = !!globalThis.describe;
+globalThis.describeDefined = Boolean(globalThis.describe);


this, and other Boolean should be != null as mentioned

github-actions · 2022-03-31T00:07:12Z

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 23, 2022

Smrtnyk reviewed Feb 23, 2022

View reviewed changes

Biki-das requested a review from SimenB February 26, 2022 15:17

Smrtnyk reviewed Feb 26, 2022

View reviewed changes

SimenB reviewed Feb 26, 2022

View reviewed changes

packages/jest-mock/src/index.ts Outdated Show resolved Hide resolved

Biki-das requested a review from SimenB February 26, 2022 18:52

SimenB reviewed Feb 26, 2022

View reviewed changes

Biki-das closed this Feb 28, 2022

Biki-das mentioned this pull request Feb 28, 2022

Chore (eslintrc.js) :- Turned on no-implicit-coercion rule and fixed occurrences #12511

Closed

github-actions bot locked as resolved and limited conversation to collaborators Mar 31, 2022

Chore(eslint):- Turned no-implicit-coercion rule to error and fixed all the occurrences #12476

Chore(eslint):- Turned no-implicit-coercion rule to error and fixed all the occurrences #12476

Conversation

Biki-das commented Feb 23, 2022

SimenB commented Feb 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Smrtnyk Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

Smrtnyk Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

Biki-das commented Feb 23, 2022

SimenB commented Feb 23, 2022 • edited Loading

Biki-das commented Feb 24, 2022

Biki-das commented Feb 24, 2022

SimenB commented Feb 24, 2022

Biki-das commented Feb 26, 2022

Smrtnyk Feb 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Smrtnyk Feb 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB commented Feb 26, 2022

Smrtnyk commented Feb 26, 2022

SimenB commented Feb 26, 2022

Biki-das commented Feb 26, 2022

Choose a reason for hiding this comment

github-actions bot commented Mar 31, 2022

Chore(eslint):- Turned `no-implicit-coercion` rule to error and fixed all the occurrences #12476

Chore(eslint):- Turned `no-implicit-coercion` rule to error and fixed all the occurrences #12476

Smrtnyk Feb 23, 2022 •

edited

Loading

Smrtnyk Feb 23, 2022 •

edited

Loading

SimenB commented Feb 23, 2022 •

edited

Loading

Smrtnyk Feb 26, 2022 •

edited

Loading

Smrtnyk Feb 26, 2022 •

edited

Loading