eslint-plugin-aura icon indicating copy to clipboard operation
eslint-plugin-aura copied to clipboard

Review recommended rules config for func-names and no-unused-expressions

Open touzoku opened this issue 6 years ago • 12 comments

Currently 'func-names': ['error', 'always'] and 'no-unused-expressions': 'error' produce errors for a perfectly valid aura controller code as below:

({
    myAction: function (component, event, helper) {
        return false
    }
})

This issue requires research on how these rules could be improved (in lieu of just disabling them).

touzoku avatar Oct 01 '19 05:10 touzoku

Hi @touzoku

Basically, these warnings come from the fact we should not have anonymous function (to increase traceability on exception for example).

The code below could be converted to:

({
    'myAction': function myAction(component, event, helper) {
        return false
    }
})

And the warnings do not appear again (I use this format now and I have no warnings / errors).

An interesting feature could be to remove the following errors (from controllers, helpers, renderers...):

  1. "Expected an assignment or function call and instead saw an expression) on first line
  2. "Missing semicolon" on last line

What are your thoughts?

Regards

rochejul avatar Oct 01 '19 11:10 rochejul

I think it is better to keep the number of false positives to the minimum. So if some rule is producing noise, it is a good candidate for demotion.

Side topic: @pmdartus is there any chance you know @rochejul or could meet in person? I think this plugin deserves a place in VSCode under /aura/ directory next to eslint-config-lwc.

touzoku avatar Oct 02 '19 06:10 touzoku

I tried to align rules with the LWC plugin. So we should be careful.

Moreover, in this case, this is not really (on my point of view) noise, because this is hardly recommended to avoid anonymous function.

It depends if we want to have as much as much possible similar rules between LWC and Aura (most of the rules are not directly related to ES6 or LWC, but on most used coding guideline, like Airbnb).

I will let you to decide this point

Regards

rochejul avatar Oct 02 '19 15:10 rochejul

A working aura controller js gives me 58 Problems of which I understand a few, if at all.

I think it is better to keep the number of false positives to the minimum.

Agreed. If a working component comes from production or is copied from documentation and I get a lot of "Problems" this is just noise to me.

If nothing else, please explain your rationale in the documentation around the Tooling for the Salesforce VSCode Plugins. Please be mindful of the fact, that successful AURA Developers might not have any previous experience with Javascript Tooling and its current standards.

Szandor72 avatar Aug 18 '20 09:08 Szandor72

My two cents: We should be able to write Aura code according to Aura documentation and get zero warnings. We're not following standards in Aura, because it's not defined to the standards, it's a special variant of JavaScript as far as I'm concerned. Also, traceability is not important, as it is transpiled anyways, easy to debug, and the trailing semi-colon is pointless and asymmetric. I already have over 23,000 PMD warnings that are actual code issues, I don't need another 10,000+ warnings cluttering up my list of things to fix. I'm pretty sure most orgs would rather ignore these warnings than fix them, and this means, if we turn off these warnings, we might miss real violations of these rules. Remember the story of the boy who cried wolf. We don't want to ignore the real wolves because of all the "fake" warnings.

brianmfear avatar Aug 18 '20 14:08 brianmfear

I'm digging into to this and seeing if we should change some of these default rules, but my general advice is that every team/company should have their own rules. Just use what works for you. It is easy to publish a shared config that your team can standardize on: https://eslint.org/docs/developer-guide/shareable-configs

ntotten avatar Aug 18 '20 14:08 ntotten

+1 for Brian that's exactly how I feel. Aura is special but that's no surprise.

My perfect solution would be: I install standard VS Code extensions for Salesforce, I create an Aura CMP, I copy stuff from docs and I get 0 problems in VS Code.

Szandor72 avatar Aug 18 '20 15:08 Szandor72

I think there are two things here.

Frist, the rules func-names and no-unused-expressions are probably aggressive. I am supportive of disabling them by default in VS Code to reduce the noise.

The second issue is a bit more complex. I get that you have a lot of warnings in an old codebase, but I don't really view this as a problem. What I would suggest is that you change your eslint rules to enforce the rules you want to actually use. Code written without ESLint is always going to have a bunch of issues when you enable it - both on and off the salesforce platform. I think there is value in encouraging developers to write more maintainable code by using ESLint, but is there value in rewriting a legacy codebase to fix warnings - probably not. In that case you should either disable ESLint or change your config to match your codebase.

ntotten avatar Aug 18 '20 15:08 ntotten

Proposed eslint rule change in templates: https://github.com/forcedotcom/salesforcedx-templates/pull/186

ntotten avatar Aug 18 '20 15:08 ntotten

I just tried those rules in the very same repo that gave raise to my initial comment.

Much better. Thank you, Nathan!

Szandor72 avatar Aug 18 '20 16:08 Szandor72

Is there any update to this issue?

jlawrence124 avatar Feb 27 '23 18:02 jlawrence124

Just stumbled on this, any updates? It's clearly a false positive in Aura controller/helper context

pkozuchowski avatar May 18 '24 20:05 pkozuchowski