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

Add TypeScript linting support #5641

Closed
3 tasks
Timer opened this issue Oct 31, 2018 · 24 comments · Fixed by #6513
Closed
3 tasks

Add TypeScript linting support #5641

Timer opened this issue Oct 31, 2018 · 24 comments · Fixed by #6513

Comments

@Timer
Copy link
Contributor

Timer commented Oct 31, 2018

We should lint TypeScript files and try to approximate to match our ESLint configuration as close as possible.

TSLint rules we want:

maybe:

@copiali
Copy link

copiali commented Oct 31, 2018

+1 Really need support for tslint.....

@Vinnl
Copy link
Contributor

Vinnl commented Oct 31, 2018

I think TSLint also has a few that can catch errors that cannot be supported by ESLint, such as no-floating-promises - wouldn't it be a good idea to catch as many potential errors as possible, even if they diverge from the ESLint configuration?

@ianschmitz
Copy link
Contributor

TSLint React Hooks: palantir/tslint-react#186

@christianchown
Copy link

@copiali for now, you can add linting manually:

  • yarn add tslint tslint-react tslint-whatever-else-you-use
  • tslint.json in your root
  • "lint": "tslint -c tslint.json src/**/*.{ts,tsx}" in your package.json scripts (I also add --fix --format verbose)

and then yarn lint

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 2, 2018

There was a discussion around tslint vs typescript-eslint-parser here. I personally prefer eslint as it has many more features, but from using typescript-eslint-parser I fear it isn't ready.

If we could get this in, with close to parity on the eslint config, I think it'd be a great solution.

Can I help with this effort?

@ianschmitz
Copy link
Contributor

@mrmckeb we're well under way adding support for this. It will be part of the 3.0 release.

See https://github.com/ianschmitz/create-react-app/tree/tslint and give it a try!

@ianschmitz ianschmitz mentioned this issue Nov 3, 2018
4 tasks
@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 4, 2018

I'll take a look, thanks @ianschmitz. In the interim we'll use the WIP version.

@harrysolovay
Copy link

I just published a working solution. Please check out Rescripts (specifically the use-tslint-config rescript). And please let me know what you think of it, how it could be improved, feature requests, criticisms, etc... I'd really love to hear from y'all! :) hope this helps

@SlashArash
Copy link

SlashArash commented Jan 21, 2019

you can use craco to rewrite cra configuration so you can add tslint-loader to webpack and use tslint.json file.

  1. install dependencies
    npm i -D @craco/craco tslint-loader tslint tslint-react

  2. change package.json to use craco

{
  "scripts": {
    "start": "craco start",
    "build": "craco build && npm run deploy",
  },
  
  1. craco.config.js
module.exports = {
  webpack: {
    configure: {
      module: {
        rules: [
          {
            test: /\.(ts|tsx)$/,
            enforce: 'pre',
            use: [
              {
                loader: 'tslint-loader',
                options: {
                  tsConfigFile: 'tsconfig.json',
                },
              },
            ],
          },
        ],
      },
    },
  },
};
  1. tslint.json
{
  "extends": ["tslint:recommended", "tslint-react"],
  "rules": {
     // any rules you need
 }
}

@PutziSan
Copy link

Check the update by @ianschmitz in the PR: #5697 (comment)

I think it would be better to integrate https://github.com/typescript-eslint/typescript-eslint when it is released and documented.

@Gelio
Copy link

Gelio commented Feb 3, 2019

  • react hooks plugin

If anyone is looking for such a plugin, I have created a rule similar to eslint-plugin-react-hooks.

Check it out:

https://github.com/Gelio/tslint-react-hooks

Feel free to post your feedback 📝 😄

@harrysolovay
Copy link

@Gelio this looks fantastic. Good work!

@mrmckeb
Copy link
Contributor

mrmckeb commented Feb 3, 2019

Hi all, it looks like Microsoft is pushing full-steam towards ESLint. This config is great, and I would recommend people to use it for the interim - but I wanted to offer warning that this may not get merged if we decide to go in that direction.

Further reading:

@kumar303
Copy link

kumar303 commented Feb 6, 2019

Just to illustrate the eslint approach as suggested up above, I was able to start a new create-react-app project with --typescript, run yarn eject (don't do this, obviously), and add this config below the existing eslint-loader in config/webpack.config.js to start seeing lint errors in console / browser:

        {
          test: /\.(ts|tsx)$/,
          enforce: 'pre',
          use: [
            {
              options: {
                formatter: require.resolve('react-dev-utils/eslintFormatter'),
                eslintPath: require.resolve('eslint'),
                parser: require.resolve('@typescript-eslint/parser'),
              },
              loader: require.resolve('eslint-loader'),
            },
          ],
          include: paths.appSrc,
        }

There is a warning about mismatched TypeScript versions so that's one thing to solve.

@Ciaran0
Copy link

Ciaran0 commented Feb 8, 2019

@kumar303 is the best way to apply that change through Rescripts?

I feel like linting config and rules really should be customisable.

@Hotell
Copy link

Hotell commented Feb 8, 2019

as @mrmckeb said. TSLint is going bye bye as the "standard" TypeScript linter. I've started to migrate all projects to eslint and it works very well.

https://twitter.com/martin_hotell/status/1093174635015880705?s=20

What should be done is to incorporate plugin:@typescript-eslint/recommended within webpack config in CRA. I can submit PR if maintainers give me green light.

@kumar303
Copy link

kumar303 commented Feb 8, 2019

@Ciaran0 I'm not familiar with Rescripts but yarn eslint configured like this in a non-ejected create-react-scripts app (in TypeScript) is working fine for us.

@Hotell
Copy link

Hotell commented Feb 8, 2019

@Ciaran0 👌 nit: "eslint": "eslint src/**/*.{ts,tsx,js}"

Yup I'm using similar approach. Anyways IMHO having run linters as pre-hook within webpack is big mistake (especially if you use typescript as you code won't compile if it contains compilation === most of the runtime errors )

@ianschmitz
Copy link
Contributor

ianschmitz commented Feb 8, 2019

@Hotell I'll transition my existing TSLint PR to use ESLint. We just need to make sure it's ready for prime time before shipping it in CRA.

@Hotell
Copy link

Hotell commented Feb 8, 2019

@ianschmitz

We just need to make sure it's ready for prime time before shipping it in CRA.

To make sure what is ready for prime time? Eslint for TypeScript ?

Everything works as expected, I would push it ASAP, because as for now, CRA TS users have no linting support at all, just useless recompile slowdown (eslint pre-hook in wp running eslint )

@mrmckeb
Copy link
Contributor

mrmckeb commented Feb 10, 2019

Yes, @Hotell :) We're just being cautious as the typescript-eslint project is very new, and we'd hate to introduce an experience that introduced more issues than it solved.

@ianschmitz
Copy link
Contributor

We've created #6513 to add support for linting via typescript-eslint.

@ianschmitz ianschmitz changed the title Add TypeScript linting via TSLint Add TypeScript linting support Feb 25, 2019
@hugoeanogueira
Copy link

@Hotell

@Ciaran0 👌 nit: "eslint": "eslint src/**/*.{ts,tsx,js}"

Using your command, I end up having troubles with some projects that don't have any JS files inside the src, with eslint reporting the following:

No files matching the pattern "src/**/*.js" were found.

Looks like eslint will break if it doesn't find any file for each of the extensions in the glob. It was hard to debug, because the command ran fine in other projects :p

The following command worked for me, ignoring missing patterns.

eslint src --ext ts,tsx,js,jsx

@nareshbhatia
Copy link

nareshbhatia commented Mar 8, 2019

I have added tslint to my project as suggested by @copiali above. However it requires to run tslint manually. Is there any way to run the tslint on file save, preferably by react-scripts? I could add some kind of watcher that would run the linter in a separate shell, but that is very awkward. Is there some kind of a hook in react-scripts that would allow me to do this more elegantly?

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

Successfully merging a pull request may close this issue.