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

Update doc on no-unused-expressions #11169

Closed
snooopcatt opened this issue Dec 7, 2018 · 1 comment · Fixed by #11192
Closed

Update doc on no-unused-expressions #11169

snooopcatt opened this issue Dec 7, 2018 · 1 comment · Fixed by #11192
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation rule Relates to ESLint's core rules

Comments

@snooopcatt
Copy link

Hello.
I think no-unused-expressions doc article could have a note, explaining why is that rule actually matters. Let me tell you a pretty dramatic story with a happy end (short version, for those not believing in fairy tales, is below).

Long version

Once upon a time there was a team of developers who adopted eslint to watch after their young Code. Everything went well, until one day developers decided to utilize fancy ecma getters/setters. And those getters were actually smart and state-changing: if property didn't exist, getter would create one. And they used to trigger getter just by refencing it:

get bar() {
  if (!this._bar) {
    this._bar = 'bar';
  }
  return this._bar;
}

foo() {
  this.bar;
}

ESLint saw this and told them: I won't let you commit this unused-expression noncense! Developers were wondering, what is wrong with this Code? They were new to getters, they did have problem with such form of expression, but they just assumed ecma allows it. So they just calmed down ESLint with an incantation:

foo() {
  // eslint-disable-next-line no-unused-expressions
  this.bar;
}

As they were saying this, ESLint shouted I CURSE YOU! A DAY SHALL COME AND YOU WILL REMEMBER MY WORDS!. Frightened, developers covered their Code with a blanket, woven of the decent test and soon forgot about very scary incident.

Time flew by, and many moons later developers decided they want to try out Angular framework, to see if their Code can play with it. They built a playground, let Angular and Code in, and watched. It was going very well, until one time, they decided to finish this playground by another incantation:

ng build --prod

and as soon as they finished incantation and looked at playground, they saw (OH NO) Code couldn't run! Couldn't even walk!

Developers cared very much about their growing Code, and were eager to help it. So they used ancient magic to see why Code didn't work. They spent hours trying to find cure, they used maps of the production application, but maps didn't show anything useful. Hopes were fading. But then one very old and wise developer, who saw Code being born, noticed that some parts of code were behaving as they were missing key logic. He found few pieces missing, and understood smth: those pieces reeked of magic! There it was, the incantation to silense ESLint, but also it was a curse. It turned out ESLint was right all the way, developers casted a reverse spell, waking up ESLint, they wrapped getter into the dummy function and (OH THE MIRACLE) playground was finished now, built in producton mode and Code was playing joyfully! Developers apologized to ESLint and vowed to be more respectful to it in future. They redid the blanket and used even more decent tests to wove it.
And they lived happily ever after.

Short version:

When we build angular demo in production mode, default setting remove dead code, so snipet below

get bar() {
  if (!this._bar) {
    this._bar = 'bar';
  }
  return this._bar;
}

foo() {
  this.bar; // <- no-unused-expression rule complains
}

will be compiled to smth like:
foo() {}

Although rule has severity level of error, it was not clear why. Code was working perfectly, so we silenced the rule. Then we got very difficult problems to debug in production code.

Conclusion

If you would add a noticeable note to the doc page, saying smth like such code might be deleted during build by some tools (e.g. ng build --prod) it would save hundreds of hours debugging.
Thank you.

@snooopcatt snooopcatt added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Dec 7, 2018
@not-an-aardvark
Copy link
Member

Hi, thanks for creating an issue. Would you be interested in creating a pull request with the suggested documentation changes?

@not-an-aardvark not-an-aardvark added rule Relates to ESLint's core rules documentation Relates to ESLint's documentation and removed core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Dec 8, 2018
platinumazure pushed a commit that referenced this issue Dec 22, 2018
* Docs: add a note for no-unused-expressions (fixes #11169)

* Docs: polish no-unused-expressions
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jun 21, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jun 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants