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

Preset of lint used in JavaScript Projects #49

Open
Keith-CY opened this issue May 10, 2023 · 6 comments
Open

Preset of lint used in JavaScript Projects #49

Keith-CY opened this issue May 10, 2023 · 6 comments

Comments

@Keith-CY
Copy link
Member

There're various JavaScript projects maintained by our team, and they follow different rules due to project history, team preference, etc.

To improve consistency, we'd better define a preset rule configuration as the initial one used in all JavaScript projects.

Do you have any suggestions? @Magickbase/developers

@yanguoyu
Copy link

yanguoyu commented May 11, 2023

  1. prettier https://prettier.io/docs/en/options.html
{
  printWidth: 120,
  // Specify the number of spaces per indentation-level.
  tabWidth: 2,
 // Indent lines with tabs instead of spaces.
  useTabs: true,
 // Only add semicolons at the beginning of lines that [may introduce ASI failures](https://prettier.io/docs/en/rationale.html#semicolons).
  semi: false,
 // Use single quotes instead of double quotes.
  singleQuote: true,
 // Trailing commas where valid in ES5 (objects, arrays, etc.). No trailing commas in type parameters in TypeScript.
  trailingComma: 'es5',
 // Print spaces between brackets in object literals. 👍 { foo: bar } 👎 {foo: bar}
 bracketSpacing: true,
  👍/**
    <button
      onClick={this.handleClick}
    >
      Click Here
    </button>
  */
 👎
    <button
      onClick={this.handleClick}>
      Click Here
    </button>
  */
 bracketSameLine: false,
 // 👍 x => x   👎 (x) => x
 arrowParens: false,
 endOfLine: 'lf'
}
  1. eslint https://eslint.org/docs/latest/rules/
    Eslint rules are so many so we can extend other rules and change some if we need.
  1. style-lint https://stylelint.io/user-guide/rules
    We can use its recommend rules https://www.npmjs.com/package/stylelint-config-recommended and change some if we need.
  2. commit-lint https://commitlint.js.org/#/
import type {UserConfig} from '@commitlint/types';
import { RuleConfigSeverity } from "@commitlint/types";

const Configuration: UserConfig = {
  /*
   * Resolve and load @commitlint/config-conventional from node_modules.
   * Referenced packages must be installed https://commitlint.js.org/#/reference-rules
   */
  extends: ['@commitlint/config-conventional'],
  /*
   * Resolve and load @commitlint/format from node_modules.
   * Referenced package must be installed
   */
  formatter: '@commitlint/format',
  /*
   * Whether commitlint uses the default ignore rules.
   */
  defaultIgnores: true,
  /*
   * Custom URL to show upon failure
   */
  helpUrl: 'https://github.com/conventional-changelog/commitlint/#what-is-commitlint',
};

module.exports = Configuration;

@WhiteMinds
Copy link

We can establish a repository to implement packages like @magickbase/tools and place common configuration files such as prettierrc.js, eslintrc.js inside it. Then, these configurations can be imported into the projects that need to use them, similar to the following example:

// .prettierrc.js
module.exports = {
  ...require('@magickbase/tools/prettierrc'),
  // other config
}

Reaching a consensus on a good solution may take a long time and involve significant differences of opinion. Therefore, we can start by incorporating the rules on which there is no disagreement into this repository and put them into practical use. This approach may facilitate the overall standardization and subsequent rule improvements.

// 👍 x => x 👎 (x) => x
arrowParens: false,

I used to think the same way, but after referring to the explanation in https://prettier.io/docs/en/options.html#arrow-function-parentheses, I believe setting it to the default value 'always' might be better. Here's the quote:

At first glance, avoiding parentheses may look like a better choice because of less visual noise. However, when Prettier removes parentheses, it becomes harder to add type annotations, extra arguments or default values as well as making other changes. Consistent use of parentheses provides a better developer experience when editing real codebases, which justifies the default value for the option.

@yanguoyu
Copy link

IMO, Type Inference is a good feature with typescript.
Should we set explicit-function-return-type error or warning? @Magickbase/developers
Magickbase/neuron-public-issues#140 (comment)

@WhiteMinds
Copy link

IMO, Type Inference is a good feature with typescript. Should we set explicit-function-return-type error or warning? @Magickbase/developers Magickbase/neuron-public-issues#140 (comment)

I lean towards setting it to 'error'. Explicitly setting the return value allows for more effective type checking and makes the return value of the function easier for code readers to understand.

@Keith-CY
Copy link
Member Author

Keith-CY commented Jun 4, 2023

IMO, Type Inference is a good feature with typescript. Should we set explicit-function-return-type error or warning? @Magickbase/developers Magickbase/neuron-public-issues#140 (comment)

I lean towards setting it to 'error'. Explicitly setting the return value allows for more effective type checking and makes the return value of the function easier for code readers to understand.

My personal thought is to set external interfaces typed explicitly while leaving the internal ones inferred, it guarantees typesafe to users while keeping the flexibility of JavaScript.

@Keith-CY
Copy link
Member Author

I'll collect prettier/eslint configurations from our projects here, and deduplicate them.

Once the checklist is ready, we can keep those controversial rules intact and expose the unanimous in the tools repo.

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

No branches or pull requests

3 participants