Relax @typescript-eslint/no-magic-numbers

[darkbasic]

Simple tasks like initializing a loop variable to zero with @typescript-eslint/no-magic-numbers current expects that you should create a separate const for zero.
This doesn't happen with @typescript-eslint/no-magic-numbers by default: https://typescript-eslint.io/play/#ts=5.5.2&fileType=.tsx&code=MYewdgzgLgBAsgQQBowLwwIwAYsG4BQ%2BAZiAE4wAUANgKawCWaMeMjAPPMrqwNQ8CUMAN74YMKAAtSIAO4wwNOQFFS00hX4EAvkA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1oFsBDAc0plaTWNwBGiaKgyQA7r2hNI4MAF8QaoA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false

I don't think the current eslint-config-love defaults for that rule are acceptable and apparently I'm not the only one that thinks so:

[mightyiam]

Sorry and thank you. Will take a look.

[eslint-config-love-release-bot]

🎉 This issue has been resolved in version 84.1.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[darkbasic]

@mightyiam unfortunately it's still not fixed even in latest version:

[mightyiam]

@darkbasic may I please see one of those?

[darkbasic]

@mightyiam just some of them:

[mcereijo]

I'm facing the same problem with version 89.0.0 and it makes the code much verbose:

expect(fetchMock).toHaveBeenCalledTimes(1)

Should be changed to:

const timesCalled = 1
expect(fetchMock).toHaveBeenCalledTimes(timesCalled)

[darkbasic]

Yeah this is insane. I get what the rule is meant to achieve, but currently it's doing more harm than good in my opinion and I ended up disabling it in all my projects.

[gtbuchanan]

For what it's worth, it's probably better to set ignore: [0, 1, 2, 100] to account for the accepted uses of magic numbers. See here for C# variant.

[darkbasic]

We should add -1 as well, since it's the "index not found" value and can be used in length - 1 as well.

[mightyiam]

This configuration is indented to be quite strict.
Please see a readme section we've just added:
https://github.com/mightyiam/eslint-config-love/blob/01af4999a739cb4ec8c51e67c63f0bf3431f5246/README.md#disabling-rules

@mightyiam just some of them:

This looks like it may be a legitimate match for this rule. Perhaps something like

const HEADER_ROW_INDEX = 0
const row = rows[HEADER_ROW_INDEX]

Granted, there's a different issue with this, #1830. I will discuss there.

Seems like another possible match for this rule. What does 1 pixel stand for? Why one pixel? Perhaps:

const BORDER_PIXEL_WIDTH = 1
header.width = newValue - BORDER_PIXEL_WIDTH

Perhaps a utility isEmpty function?

This also looks to me like it could use encapsulation as a utility function, in which an eslint-disable comment would be appropriate.

expect(fetchMock).toHaveBeenCalledTimes(1)

I would disable the rule in the test files in which counts are used. Or maybe all of the test files.

For what it's worth, it's probably better to set ignore: [0, 1, 2, 100] to account for the accepted uses of magic numbers. See here for C# variant.

I'm concerned that ignoring specific numbers would reduce the efficacy of the rule.

[darkbasic]

Please see my comment on the other thread: #1830 (comment)