search

benawad / codeponder

Marketplace for Code Reviews
279 stars 49 forks source link

Linting Codeponder - TSLint soon to be deprecated #287

Open onemen opened 5 years ago

onemen commented 5 years ago

Tslint

tslint is installed but i did not find any configuration file or script to run it

Testing tslint with the configuration below produce many linting errors and warnings.

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

Eslint + typescript-eslint

TSLint Deprecated to Focus Support on typescript-eslint

Testing eslint with the configuration below produce many linting errors and warnings.

module.exports = {
  "root": true,

  "parser": "@typescript-eslint/parser",

  "plugins": ["@typescript-eslint"],

  "extends": [
    "react-app",
    "plugin:@typescript-eslint/recommended",
    "prettier",
    "prettier/@typescript-eslint"
  ],

  "env": {
    "es6": true,
    "node": true,
  },

  "parserOptions": {
    "sourceType": "module",
    "ecmaVersion": 2018
  },

  "rules": {
    "no-useless-constructor": 0
  },

  "globals": {  }
};

I think you should replace tslint with eslint and configure it for react/typescript project with prettier. You can also install vscode extension for Eslint.

benawad commented 5 years ago

Sounds good

onemen commented 5 years ago

The tables below contains list of linting issues when applying recommended rules from typescript-eslint and react-app.

I can work on PR to fix those issues.

I thinks we need to fix all react-app linting issues, regarding typescript-eslint let me know if you want to fix all the issues from the recommended rules or if you want to allow some.

Let me case you want to enable other rules.

@typescript-eslint/eslint-plugin Issues Files
@typescript-eslint/array-type 2 1
@typescript-eslint/ban-types 3 2
@typescript-eslint/camelcase 24 3
@typescript-eslint/class-name-casing 1 1
@typescript-eslint/explicit-function-return-type 101 60
@typescript-eslint/explicit-member-accessibility 133 32
@typescript-eslint/interface-name-prefix 1 1
@typescript-eslint/no-empty-interface 1 1
@typescript-eslint/no-explicit-any 62 34
@typescript-eslint/no-non-null-assertion 58 19
@typescript-eslint/no-object-literal-type-assertion 1 1
@typescript-eslint/no-parameter-properties 11 5
@typescript-eslint/no-unused-vars 5 5
@typescript-eslint/no-use-before-define 4 2
@typescript-eslint/no-var-requires 3 1
@typescript-eslint/prefer-interface 4 4
eslint-config-react-app Issues Files
array-callback-return 1 1
default-case 1 1
eqeqeq 8 1
import/first 23 1
jsx-a11y/alt-text 1 1
jsx-a11y/anchor-is-valid 12 8
no-sparse-arrays 1 1
no-useless-constructor 5 5
react/jsx-no-target-blank 1 1
react/react-in-jsx-scope 25 5
benawad commented 5 years ago

I'm cool with you fixing all of them

onemen commented 5 years ago

While working on fixing lint error I've noticed that some files are not in use. let me know if this files cane be remove

benawad commented 5 years ago

yeah you can remove them

onemen commented 5 years ago

I've found more file that maybe are not in use, please let me know if I can remove these files. If some of the files are in use, I hope you can point me to the place that use them.

Packages\web

Packages\server

I'm not an expert in graphql so maybe I've missed the place that use these files. Maybe some other files are generated from these files ?

benawad commented 5 years ago

Web

everything in the pages folder is used. They are routes.

We will use these in the future

You can remove

Server

All the resolver.ts files are used here https://github.com/benawad/codeponder/blob/master/packages/server/src/index.ts#L48

You can remove