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

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

Open
touzoku opened this issue Oct 1, 2019 · 12 comments
Open

Comments

@touzoku
Copy link
Contributor

touzoku commented Oct 1, 2019

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).

@rochejul
Copy link
Collaborator

rochejul commented Oct 1, 2019

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

@touzoku
Copy link
Contributor Author

touzoku commented Oct 2, 2019

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.

@rochejul
Copy link
Collaborator

rochejul commented Oct 2, 2019

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

@Szandor72
Copy link

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.

@brianmfear
Copy link

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.

@ntotten
Copy link

ntotten commented Aug 18, 2020

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

@Szandor72
Copy link

+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.

@ntotten
Copy link

ntotten commented Aug 18, 2020

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
Copy link

ntotten commented Aug 18, 2020

Proposed eslint rule change in templates: forcedotcom/salesforcedx-templates#186

@Szandor72
Copy link

Szandor72 commented Aug 18, 2020 via email

@jlawrence124
Copy link

Is there any update to this issue?

@pkozuchowski
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants