Linting the server side code

[DedrickEnc]

This PR request amends the code by correcting errors of syntax in most of files in the server side based on the eslint rules.
Some rule was edited directly in the .eslintrc.js

references #1429

Thank you for contributing!

Before submitting this pull request, please verify that you have:

• Run your code through ESLint. Check out our styleguide.
• Run integration tests.
• Run end-to-end tests.
• Accurately described the changes your are making in this pull request.

For a more detailed checklist, see the official review checklist that this PR will be evaluated against.

Ensuring that the above checkboxes are completed will help speed the review process and help build a stronger application. Thanks!

[sfount]

This pull request contributes to #1429

[ghost]

☔ The latest upstream changes (presumably 0856853) made this pull request unmergeable. Please resolve the merge conflicts.

[jniles]

In my experience, this rule is very helpful in cleaning up my code. The only example I can think of that makes it annoying is ExpressJS error handlers, where functions must have arity 4 (and so you might not use next, but are required to pass it in).

What cases are you finding that it would be useful to turn it off?

[DedrickEnc]

In the express, when passing next as parameter but not use that. that why I disabled this.

[jniles]

next isn't required. The following code is perfectly valid expressjs middleware:

app.get('/route', (req, res) => res.redirect('/other/route'));

If code doesn't use next, it is confusing to pass it into a function. A better fix would be to simply remove next from the parameters.

As mentioned about, if it is error handling middleware (which we should only have one), we can just put comment telling ESlint to ignore that line.

[jniles]

In my opinion, we should enable func-names, perhaps with the "as-needed" parameter. Naming a function is better for logs and debugging, which is super helpful in production -- or at least until we have a better logger (like #1519).

I think this should take care of all places that the name cannot be assumed. To understand how Javascript figures out a function name, Dr. Axel Rauschmayer has a really detailed article on it.

[DedrickEnc]

@jniles
I agree, but this rule lead me to even a function which process a promise

.then(function (){
});

produces an error

.then(function someName(){
});

passes but produces an error in the execution

So do you think, I should declare those function on the top of their scopes?

[jniles]

Interesting ... I have never tried .then(function someName() { /* ... */}) before.

It seems to me that callbacks do not need access to this, so they can be written more easily as arrow functions:

.then((data) => {
  // do something without declaring this
});

That way the function's scope is the parent's scope and it behaves like any other code block.

[DedrickEnc]

@jniles
Conflicts are fixed

[jniles]

I still get 456 lint errors, so the server work is not completed, but this is already a huge help!

Code changes look good to me. 👍

[jniles]

What's going on here?

[jniles]

@kwilu r+

[ghost]

📌 Commit 171343a has been approved by jniles

[ghost]

⌛ Testing commit 171343a with merge 5e5a3c8...