Add ESLint to Project

[nicholaschuayunzhi]

Adds ESLint and its lodash plugin as a dev dependency. --fix command not executed yet. The eslintrc file was taken from https://github.com/MarkBind/markbind/blob/master/.eslintrc.js

[Gisonrg]

Nice one! Can't wait to fix all the style issues in the project :D

Before I merge this, do you think we should use an existing style standard for our eslint?
I am thinking about using either https://github.com/airbnb/javascript or https://github.com/google/eslint-config-google

Feel free to discuss with the team to choose a standard you all agreed on.

[nicholaschuayunzhi]

I've used airbnb before. I would prefer airbnb as their docs have the eslint rules which we can ctrl f easily to see an example of.

[damithc]

The common standards for our projects (open for updates) https://github.com/oss-generic/process/blob/master/docs/CodingStandards.adoc

[Gisonrg]

Let's use the airbnb preset and modified based on our standard then :P

@nicholaschuayunzhi Can you help us set up the eslint as discussed?

[acjh]

The common standards for our projects (open for updates) https://github.com/oss-generic/process/blob/master/docs/CodingStandards.adoc

If a topic is not covered in this document, refer to airbnb JavaScript Style Guide

airbnb it is! Also, teammates-eslint.yml.

[nicholaschuayunzhi]

Okay I'll work on it. Need to do some research first.

Also do you want it to be standardized across all the modules?

[Gisonrg]

Nice, take you time. Let's start from markbind-cli first, then extend it to markbind (vue modules has different style standard).

[nicholaschuayunzhi]

Changed rules wrt to oss-generic coding standards

Rules that can't be modified with existing eslint rules:

• Constants named in screaming snake case
• Prefer higher-level breaks to lower-level breaks. In the example below, the first is preferred, since the break occurs outside the parenthesized expression, which is at a higher level.
• suggestion for ternary expression
  

Read up on writing custom rules. Is it worth the time? Anyone with experience? Also will add to dev guide on how to set up eslint.

[nicholaschuayunzhi]

I've updated the eslint config as discussed. Disabled func-names rule and added line break style.

[acjh]

Alphabetical (ascii) order:

• func-names before function-paren-newline
• FunctionDeclaration before FunctionExpression

[acjh]

1 seems to be the default in indent and is not overridden by airbnb.

[acjh]

I took a while to understand why some choices were made.

Perhaps we should document this in the Wiki, together with the setup instructions.

[acjh]

Alphabetical (ascii) order:

• FunctionDeclaration before FunctionExpression

[acjh]

Perhaps we should document this in the Wiki, together with the setup instructions.

Maybe not with the setup instructions (seems out-of-place in the Developer Guide).

[nicholaschuayunzhi]

I'm working on the explanation in a separate gist i can link to in the dev guide. Sorry for missing out the previous ordering of keys.

[nicholaschuayunzhi]

I've removed ArrayExpression option in indent as the default option (1) account for aligning elements based on 2 spaces of indentation.

I've added the "array-bracket-newline": ["error", { "multiline": true }], to be consistent with airbnb's
object-curly-newline rule, as well as to prevent such code from passing which is not wanted based on nus oss js standard.

let array = [1,
  2,
  3,]

[nicholaschuayunzhi]

I've added a gist for the explanation to the developer guide.

[acjh]

to be consistent with airbnb's object-curly-newline rule

I don't see this 🤔

[acjh]

Remove space before comma.

[acjh]

I've added a gist for the explanation to the developer guide.

Nice. This can be a page in the Wiki. Let's create a sidebar; the current pages need categorisation anyway.

[nicholaschuayunzhi]

to be consistent with airbnb's object-curly-newline rule

I don't see this 🤔

Oh dear, i think i saw wrongly. But the rationale still stands to prevent the below from happening.

let array = [1,
  2,
  3,]

Weird though, my linter is firing off that error. Could it be a default rule? https://eslint.org/docs/rules/object-curly-newline

[acjh]

Both rules seem to be { "multiline": true } by default.

[nicholaschuayunzhi]

Seems when i do not specify array-bracket-newline, it does not fire off any linting error.

Need to look into it further.

[nicholaschuayunzhi]

In the eslint docs for both rules, seems like array-bracket-newline is not enabled by default.

However for object-curly-newline. It is enabled.

So i think we should still have the array-bracket-newline rule.

[acjh]

Nice catch.

[nicholaschuayunzhi]

I still find it weird though, according to user guide, no rules are enabled by default, but the docs seem to say otherwise.

[nicholaschuayunzhi]

Ok i think i got it, they disabled array-bracket-newline and enabled object-curly-newline in airbnb-base preset. And it is not documented on the readme.

https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb-base/rules/style.js

[nicholaschuayunzhi]

Okay I think it should be ready for review. Note that operator-linebreak is manually enabled as airbnb-base package with enabled operator-linebreak rule is not published yet

I've also removed the line endings rule as airbnb-base already checks for unix endings

[Gisonrg]

LGTM, feel free to merge this @acjh

[acjh]

@nicholaschuayunzhi Thanks for your contribution!

[acjh]

@nicholaschuayunzhi Can you create a PR to apply --fix?

[nicholaschuayunzhi]

@acjh Will do!