eslint: no-magic-numbers should allow passing magic numbers to specific functions like setTimeout

Code that does animations is another example of where it’s often difficult to find meaningful descriptions for numbers, and trying to have a named variable actually makes the code incredibly confusing. In many situations, the number is derived by trial and error to get a value that “feels right” rather than some thoughtful, describable process. In such scenarios, the magic number is actually the most descriptive way to convey what the number is: a magic number. Also, in these scenarios, the main focus of someone reading the code is not what the numbers mean (it’s pretty obvious due to context) but rather being able to clearly see the flow of the animation code and reason about its intent; a load of wordy number constants can actually make such code very difficult to read.

Being able to define some exceptions to the rule like suggested in OP would be beneficial in such scenarios.

I can sort of understand setYear example that you provided. Even with that I’m having a trouble convincing myself it’s not a magic number, because while it’s clear that 1984 is a year, it’s not clear why you chose 1984 and not 1882. However, why do you think 500 is not a magic number? The reason for setting timeout to 500ms is completely magical to me. Why not 502?

This rule is very strict and hard to follow 100%, but that’s exactly it’s intention. It exists to create self-documenting code. And self-documenting code requires explanation for each number you choose.

@analog-nico @aubergine10 @Slayer95 With so many members of the team not wanting to see this change, you have an uphill battle to fight to get this in. I understand the reasoning behind your request, and I think in general animation is a good example where magic numbers are OK to use, but as a team, we can’t add every exception under the sun to our rules, and we have to try to keep rule principles as pure as possible. Also, when we are talking about ESLint not being opinionated, we are not talking about every rule being infinitely configurable, we are talking about giving users the ability to turn any rule on or off at any point. I think your best bet is to copy this rule and create your own plugin that would allow exceptions.

@aubergine10 I think this is a nice proposal which deserves its own, dedicated ticket 😉

Is one aspect of the problem caused by the verbosity of eslint inline comments, making them a chore to use?

Instead of doing something like this:

/*eslint no-magic-numbers: "off"*/

There could be a shorthand + and - syntax to turn a rule on or off:

// eslint -no-magic-numbers

// eslint +no-magic-numbers

Where:

• -<rulename> means turn rule "off"
• +<rulename> means revert to previous state, otherwise state defined in config file, otherwise "error"

If we also allowed aliases of rules, it would make toggling them even more brief:

// in .eslintrc

//...
   aliases: {
     "nmn": "no-magic-numbers"
   }

An alias, when used in an inline comment, would always be prefixed with @ to avoid possibility of conflicts with other rules. Also, it would allow a minified syntax to be used:

'turn on no-magic-numbers:'
// +@nmn

'turn off no-magic-numbers:'
// -@nmn

'long rule name can be used instead of alias if desired:'
// +@no-magic-numbers
// -@no-magic-numbers

'now we have short form to refer to rule, we could do stuff like this:'
// @no-magic-numbers "error"
// @no-magic-numbers "off"

'maybe allow comma chaining, eg:'
// @one-var "warn", @no-magic-numbers "error"

If all keyboards have an § key (I’ve been Mac user for way too long), maybe that would be better than @?

This seems like it would be an excellent feature for devs looking into integrating this rule into a code base, and it also seems to be a sensible permanent setting for the rule due to the maintainability argument brought forward by @aubergine10

While I can see why no explicit number can really be regarded as non-magic, there is clearly a demand for ignoring them. Disregarding this demand in favor of the most pure version of the rule seems to go against the “no-agenda” philosophy of ESLint…

@mikesherov - an example might be a number representing angle, passed in to an setAngleDeg() method, you’d end up with something like this:

someObj.setAngleDeg(angle)
// vs...
someObj.setAngleDeg(175)

Which of those aids reasoning about the code the most? You could argue that the value 175 could be a named value south… but what if the container element of the element associated with someObj is spinning anti-clockwise at 20 radians per second - which way is south?

It’s a bit like the valid case for using i or j as counters/indexes in a loop - everyone knows what the number is about, but the cognitive focus is not so much on the number (or “the why of the number”) but instead the surrounding code… The i or j is still crucial, but there’s no point giving it a descriptive name because that would just obfuscate the surrounding code… a descriptive name would make the i or j steal the limelight from the surrounding code, and to no real benefit of the developer, the project deadline or even long-term maintainability.

It’s similar with the setAngleDeg() example above - the developer is more interested in the fact that the angle is being set than why that value was specifically chosen (they also want to see the current value right there in the code, rather than have to go searching for it in some declarations a few PgUp’s away).

The no-magic-numbers rule is still useful for the overall script - most places the rule must be observed - but being able to optionally specify functions to exclude from the rule (the exclusion list would be empty by default) would save a lot of hassle in these fringe cases.

BTW, I found a magic number in the no-magic-numbers source code: 😛

function shouldIgnoreNumber(num) {
  return ignore.indexOf(num) !== -1; // why -1? 😛 
}

There are patters where it’s clearer to have a bare number than a named value; if the -1 above was replaced with notFoundInArray it would, IMO, make it harder to scan that code and instantly deduce what it’s doing. As soon as we see -1 on the same line as .indexOf() we know the intent. The same is true for (some) setTimeout() and (most) setAngleDeg() scenarios; sometimes a bare number massively improves code comprehension.

I think it’s a great suggestion (even if I disagree) and I’m glad to have this discussion.

Even with arbitrary animation lengths, I still think NORMAL_SPEED, FAST, SLOW, etc. are easily meaningful names you can give to animation lengths.

👎 to this proposal IMO