strothj / react-app-rewire-typescript-babel-preset

Add TypeScript support to Create React App using @babel/preset-typescript
https://react-app-rewire-typescript-babel-preset.netlify.com
MIT License
52 stars 6 forks source link

Add support for TSLint #13

Closed ianschmitz closed 6 years ago

ianschmitz commented 6 years ago

I wanted to start the conversation to see if there's interest in supporting TSLint within this package. With the addition of TSLint I think this package would be a great solution for the majority of users whom want to use CRA, but wish to have TypeScript support, without having to use a forked version of CRA such as https://github.com/wmonk/create-react-app-typescript/.

Alternatively I could publish the TSLint support as separate package that would work nicely with this one.

I'd be happy to help contribute if there's interest!

strothj commented 6 years ago

Thanks for the kind words! I've enjoyed using this project in a lot of the work I do.

I like your idea, I think something like this would be good:

const {
  rewireWebpack: rewireTypescript,
  rewireJest: rewireTypescriptJest,
  rewireTSLint
} = require("react-app-rewire-typescript-babel-preset");

module.exports = {
  webpack: function(config, env) {
    let rewiredConfig = config;

    rewiredConfig = rewireTypescript(rewiredConfig);
    rewiredConfig = rewireTSLint(rewiredConfig, ...); // accept an optional second options parameter here to pass along to tslint-loader

    return rewiredConfig;
  },
  jest: function(config) {
    return rewireTypescriptJest(config);
  }
};

The rewireTSLint should remove react-scripts's ESLint loader and replace it. Also, either another example added to the monorepo or the current example extended to use it.

What do you think? If you get this working, I'll be happy to merge it in!

ianschmitz commented 6 years ago

I was thinking along those lines! We have at least 2 good options here:

Use tslint-loader as you suggested above

Pros:

Cons:

Use tslint-language-service

Pros:

Cons:

strothj commented 6 years ago

My current work flow is this:

For the tslint-language-service plugin: I've used this in the past. Since we don't actually use the TypeScript compiler (we just strip the typing syntax using the Babel plugin), this plugin would not normally get invoked. The user would need to invoke the typescript compiler using a package.json script.

I think the value we would be adding with this would be integration with the Webpack error page. If the user isn't interested in having the errors displayed in the browser during development, they can instead invoke TSLint through a script or integrate tslint into a tsc step using the language service.

If you pass along the tslint-loader options, the user will have the option to use type checking mode or not depending on their performance requirements. I have a plugin for Storybook that parses TypeScript information for components and it does cause a significant slow down (maybe my implementation is bad...).

I think the only scenario that requires changes on the part of the rewire is the one where we add tslint-loader support. Because we'll be offering it as a separate rewire, the behavior will be opt-in.

strothj commented 6 years ago

Pardon the close/reopen, fat fingered on mobile. :p