---
# System prepended metadata

title: Lint rules
tags: [build]

---

---
robots: nofollow, noindex
tags: build
---

# Lint rules

[TOC]

## Current status

|Rule|v0|v7|
|--|--|--|
|allow explicit `any` type|yes|goal is no, some packages yes|
|braces around single-line if/for/do/while|not required|required|
|member ordering|? in general, render method last|- static fields: public, protected, private<br/>- instance fields: public, protected, private<br/>- static methods: public, protected, private<br/>- public constructor, instance methods<br/>- protected constructor, instance methods<br/>- private constructor, instance methods<br/>|
|method visibility modifiers|disallowed|required|
|method/function naming|?|overall: `/^_?[a-z][a-zA-Z\d]+$/`<br/>leading underscore:<br/>- required for private methods<br/>- optional for functions, protected methods<br/>- disallowed for public/generic and public/generic static methods|
|interface naming: use `I` prefix?|no?|required|
|prefer `const` (instead of `let`)|?|yes|
|allow `var` keyword|?|no|
|allow unnecessary string literal property access (`foo['some-prop']` is necessary, `foo['prop']` is not)|yes|no|
|allow bitwise operations|yes|no (override when needed)|
|allow shadowed variable names|yes, [discussion](https://github.com/microsoft/fluent-ui-react/pull/1261)|no|
|boolean attribute values: `<Foo disabled />` vs `<Foo disabled={true} />`|?|looks like we'd intended to have a lint rule enforcing the first option, but it was incorrectly configured and hasn't been enforced (so code is inconsistent)|
|`bind` or lambdas as JSX props|?|banned|


## Proposal ❓

- For eslint rules, we'll start by extending:
  - [`airbnb`](https://www.npmjs.com/package/eslint-config-airbnb)
  - [`prettier`](https://www.npmjs.com/package/eslint-config-prettier) (just turns off rules that may conflict with prettier)
  - Some part of [`typescript-eslint`](https://github.com/typescript-eslint/typescript-eslint)
    - https://github.com/typescript-eslint/typescript-eslint/tree/master/docs/getting-started
  - [`plugin:react-hooks/recommended`](https://www.npmjs.com/package/eslint-plugin-react-hooks)
  - (do any of these have a rule to ban ts-ignore?)
  - Then disable individual rules from any of those configs as needed if we end up finding them annoying/unhelpful
- v7's tslint rules will be minimally modified until we can migrate those packages to eslint
- For both v0 and v7 "legacy" code, we'll have overrides of the new default eslint config as needed, especially for things that can't be automatically fixed and occur frequently.
- Additional questions ❓❓❓
  - export default or not
    - proposal: no for functions, maybe for components
  - prefer lambdas for functions at file scope
    - proposal: no (but up to discretion)
- Specific rules: see below

In the [survey](https://microsoft.sharepoint-df.com/:x:/t/FluentUIInternal/EQltV6-c_qNJs7Rif3sdPJIB_fa-lsl4U2EWN3WtZnviQg?e=I3Hhwu), most of the rules had a pretty strong consensus across teams unless otherwise noted.

| Rule | Consensus |
|------|----------|
|Explicit `any`|ban (override as needed)|
|Braces around single-line `if`/etc|require|
|Prefer `const`|yes (I'm the only one dissenting 😭)|
|String literal property access|ban when unnecessary|
|Shadowed names|ban|
|`bind` in JSX props|ban|
|Boolean attribute true values|omit|
|Deprecated things|ban (override as needed)|
|`I` prefix for interfaces (discussed previously)|no|

These were the contentious ones, mostly split along team lines:

|Rule|Notes|
|--|--|
|Class member ordering|❓ team split|
|Visibility modifiers|❓ team split|
|Leading underscore|❓ lack of consensus; most common was:<br/>- optional for private methods<br/>- TBD for non-exported functions<br/>- no for everything else|
|lambdas in JSX props|❓ team split|


### Class member ordering

|Option|Votes|v0 team|v7 team|
|--|--|--|--|
|Like v0|2|2|0|
|Like v7|2|0|2|
|Like v7 with exception for render|2|1|1|
|Don't care|3|2|1|
|If i have to choose between 0 or 7, I choose 7. But don't really care, whatever that's easier. we are moving towards FC anyway|1|0|1|

The last option was from Xu and is a good point: this doesn't matter so much when all new components should be functional. So maybe we should just leave the rules as-is for each version and either not have a rule in new code, or pick something for new code (airbnb config default?) and agree it doesn't hugely matter.

### Visibility modifiers

|Option|Votes|v0 team|v7 team|
|--|--|--|--|
|Required|4|0|4|
|Disallowed|3|3|0|
|Don't care|3|2|1|

This one I have a hard time understanding. When previously discussed, IIRC the reasoning given was that having visibility modifiers messes up desired member ordering (especially render method last). But if that's the case, we should just customize the member ordering rules (write our own if needed) instead.

### Leading underscore

Numbers after vote count are v0/v7

|Area|Required|Optional|Ban|Don't care|Result|
|--|--|--|--|--|--|
|Private methods|3 (1/2)|2 (1/1)|2 (2/0)|3 (2/1)|❓ **Optional?**|
|Protected methods|0|3 (1/2)|4 (2/2)|3 (2/1)|❓ **Ban?**|
|Non-exported functions|1 (0/1)|2 (1/1)|3 (3/0)|4 (1/3)|❓|
|Exported functions|0|1 (1/0)|8 (4/4)|1 (0/1)|**Ban**|
|Public methods|0|1 (1/0)|9 (4/5)|0|**Ban**|

### Lambdas in JSX

|Option|Votes|v0 team|v7 team|
|--|--|--|--|
|Yes|4|4|0|
|No|5|1|4|
|Don't care|1|0|1|

## Current status old version

(in bullet form)

- allow explicit `any` type
  - v0: yes
  - v7: goal is no, some packages yes
- braces around single-line if/for/do/while
  - v0: not required
  - v7: required
- member ordering
  - v0: 
    - ? in general
    - convention: render method last
  - v7: 
    - static fields: public, protected, private
    - instance fields: public, protected, private
    - static methods: public, protected, private
    - public constructor, instance methods
    - protected constructor, instance methods
    - private constructor, instance methods
- method visibility modifiers
  - v0: disallowed
  - v7: required
- method/function naming
  - v0: ?
  - v7: 
    - overall: `/^_?[a-z][a-zA-Z\d]+$/`
    - leading underscore:
      - required for private methods
      - optional for functions, protected methods
      - disallowed for public/generic and public/generic static methods
- interface naming: use `I` prefix?
  - v0: no I?
  - v7: require I
- prefer `const` (instead of `let`)
  - v0: ?
  - v7: enabled
  - **proposal**: ?
- allow `var` keyword
  - v0: ?
  - v7: no
  - **proposal**: no
- allow unnecessary string literal property access (`foo['some-prop']` is necessary, `foo['prop']` is not)
  - v0: yes
  - v7: no
  - **proposal**: no
- allow bitwise operations
  - v0: yes
  - v7: no (override when needed)
  - **proposal**: no (override when needed)
- allow shadowed variable names
  - v0: yes https://github.com/microsoft/fluent-ui-react/pull/1261
  - v7: no
- boolean attribute values: `<Foo disabled />` vs `<Foo disabled={true} />`
  - v0: ?
  - v7: looks like we'd intended to have a lint rule enforcing the first option, but it was incorrectly configured and hasn't been enforced (so code is inconsistent)
- `bind` or lambdas as JSX props
  - v0: ?
  - v7: banned