Skip to content
This repository was archived by the owner on Jan 26, 2019. It is now read-only.

Default tslint config has style rules #216

Closed
apihlaja opened this issue Dec 30, 2017 · 6 comments
Closed

Default tslint config has style rules #216

apihlaja opened this issue Dec 30, 2017 · 6 comments

Comments

@apihlaja
Copy link

apihlaja commented Dec 30, 2017

I changed quotation marks of App.tsx and now it doesn't pass provided tslint config, wrong quotation marks. That reveals quite significant disparity between original CRA and typescript version.

Create React App intentionally provides a minimal set of rules that find common mistakes. ref

Create React App only uses ESLint for verifying correctness, not style. ref

create-react-app 1.4.3
react-scripts-ts 2.8.0

I didn't find any documentation / discussion about the reasoning behind the difference. Ie. why does typescript version nitpick about quotation marks?

Edit: It seems original error was caused by me accidentally reformatting App.tsx but point stands.

@DorianGrey
Copy link
Collaborator

DorianGrey commented Dec 31, 2017

Ie. why does typescript version nitpick about quotation marks?

Because of this linter rule: https://github.com/wmonk/create-react-app-typescript/blob/master/packages/react-scripts/template/tslint.json#L64

You are right that this behavior is different from CRA. This topic is a bit complicated, since tslint includes several rules regarding code style by default.

Personally, I'm using a tslint config like this as a base for my projects:

{
  "extends": [
    "tslint:recommended",
    "tslint-react",
    "tslint-config-prettier"
  ]
}

The last extension, while originally intended for use with prettier, disables all rules that affect code style even if prettier is absent. Suppose something like this would be less opinionated, but still useful enough ?

@vpicone
Copy link

vpicone commented Jan 14, 2018

@DorianGrey tslint-config-prettier doesn't handle explicitly defined that @apihlaja is talking about. The most obvious of which is:

"quotemark": [true, "single", "jsx-double"],

If you extend tslint-config-prettier and leave the tslint.json as is, it will switch between single and double quotes every save/format. There's a fundamental discrepancy between the two style choices in prettier and the explictly defined rules in the tslint.json file and one of them has to be removed.

@wmonk are you using prettier in your environment? As is, I'll probably just remove the explicitly defined rules one at a time from the tslint.json when they cause issues.

Using the tslint-config-prettier CLI tool these are the affected rules:

Error: Conflict rule(s) detected in ./tslint.json:
- align
- indent
- max-line-length
- no-consecutive-blank-lines
- one-line
- quotemark
- semicolon
- typedef-whitespace
- whitespace

@vpicone
Copy link

vpicone commented Jan 14, 2018

Here is a tslint.json with all the conflicted rules removed. Note also I extend tslint-config-prettier at the top. You'll want to install it in your project:

npm i -D tslint-config-prettier
{
  "extends": ["tslint-react", "tslint-config-prettier"],
  "rules": {
    "ban": false,
    "class-name": true,
    "comment-format": [true, "check-space"],
    "curly": true,
    "eofline": false,
    "forin": true,
    "interface-name": [true, "never-prefix"],
    "jsdoc-format": true,
    "jsx-no-lambda": false,
    "jsx-no-multiline-js": false,
    "label-position": true,
    "member-ordering": [
      true,
      "public-before-private",
      "static-before-instance",
      "variables-before-functions"
    ],
    "no-any": true,
    "no-arg": true,
    "no-bitwise": true,
    "no-console": [
      true,
      "log",
      "error",
      "debug",
      "info",
      "time",
      "timeEnd",
      "trace"
    ],
    "no-construct": true,
    "no-debugger": true,
    "no-duplicate-variable": true,
    "no-empty": true,
    "no-eval": true,
    "no-shadowed-variable": true,
    "no-string-literal": true,
    "no-switch-case-fall-through": true,
    "no-trailing-whitespace": false,
    "no-unused-expression": true,
    "no-use-before-declare": true,
    "radix": true,
    "switch-default": true,

    "trailing-comma": [false],

    "triple-equals": [true, "allow-null-check"],
    "typedef": [true, "parameter", "property-declaration"],
    "variable-name": [
      true,
      "ban-keywords",
      "check-format",
      "allow-leading-underscore",
      "allow-pascal-case"
    ]
  }
}

@DorianGrey
Copy link
Collaborator

DorianGrey commented Jan 15, 2018

@vpicone - suppose we've got a misunderstanding here, because

[...] and leave the tslint.json as is

was not my intention. The intention was to replace the currently used tslint.json with

{
  "extends": [
    "tslint:recommended",
    "tslint-react",
    "tslint-config-prettier"
  ]
}

entirely. The resulting behavior would be closer to what CRA achieves with the ESLint rules it utilizes ( tslint-eslint-rules might help out here as well). This would disable all styling rules conflicting with prettier and thus with most opinionated style choices as well.

@vpicone
Copy link

vpicone commented Jan 16, 2018

@DorianGrey You're totally right, I agree the explicit rule definitions here seem out of step with CRA.

@apihlaja
Copy link
Author

I got annoyed yesterday when working first time react-script-ts project so I was looking if I should create PR for this. Apparently it's done some hours ago already, #281 👍

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

No branches or pull requests

3 participants