[frontend] Switch to TSLint

[kwasi]

/area frontend
/kind feature

Background: We have a tslint.json but lint with eslint due to react-scripts webpack

I added some generated protocol buffers in 13794fc and broke frontend webpack compile due to lint failures for the no-undef rule. I was thrown off by the fact that there is a tslint.json configuration file but the failures were an eslint rule.

Adding the generated file path to an eslintIgnore block in package.json didn’t work and adding the path to tslint.json didn’t fix the webpack compile either.

How does eslint get run?

When scripts run by create-react-app launch webpack the linting phase uses eslint-loader with the eslint-config-react-app configuration [1], which uses @typescript-eslint to provide linting rules for TypeScript files to ESlint [2] without adding a dependency on tslint.

This configuration also ignores any eslintIgnore or eslintConfig blocks in package.json unless the EXTEND_ESLINT environment variable is set [3].

Proposal: Modify the project to use TSLint through react-scripts-ts

• Using tslint will prevent programmer error when trying to make lint changes in tslint.json
• react-scripts-ts uses tslint directly instead of eslint with TypeScript support [4]
• react-scripts-ts doesn't require environment variables to apply TSLint config
• Supports aligning frontend development practices with kubeflow/pipelines which uses tslint

Tasks for implementation:

• Switch react-scripts to react-scripts-ts in package.json to pick up tslint.json
• Remove eslint configuration lines from package.json
• Format the codebase using tslint

Caveats

There are a lot of existing files in the codebase that don’t match the rules declared in tslint.json, so adding tslint will break the webpack compile until the lint errors are fixed. If we do this in conjunction with using adding prettier for code styles (#141), we can do both as one large-scale reformatting and reduce the merge overhead.

References

[1] https://github.com/facebook/create-react-app/blob/6d6dfa9ba5a6042ab5690fe2a38b3ee54e644c9f/packages/react-scripts/config/webpack.config.js#L349

[2] https://github.com/facebook/create-react-app/blob/6d6dfa9ba5a6042ab5690fe2a38b3ee54e644c9f/packages/eslint-config-react-app/index.js#L57

[3]
https://github.com/facebook/create-react-app/blob/6d6dfa9ba5a6042ab5690fe2a38b3ee54e644c9f/packages/react-scripts/config/webpack.config.js#L359

[4] https://github.com/jpavon/react-scripts-ts/blob/d80cda5b72d17aeb217b23b331d9eba225319b3c/config/paths.js#L62

[k8s-ci-robot]

@kwasi: The label(s) area/frontend cannot be applied. These labels are supported: api-review, community/discussion, community/maintenance, community/question, cuj/build-train-deploy, cuj/multi-user, platform/aws, platform/azure, platform/gcp, platform/minikube, platform/other

Details

In response to this:

/area frontend
/kind feature

Background: We have a tslint.json but lint with eslint due to react-scripts webpack

I added some generated protocol buffers in 13794fc and broke frontend webpack compile due to lint failures for the no-undef rule. I was thrown off by the fact that there is a tslint.json configuration file but the failures were an eslint rule.

Adding the generated file path to an eslintIgnore block in package.json didn’t work and adding the path to tslint.json didn’t fix the webpack compile either.

How does eslint get run?

When scripts run by create-react-app launch webpack the linting phase uses eslint-loader with the eslint-config-react-app configuration [1], which uses @typescript-eslint to provide linting rules for TypeScript files to ESlint [2] without adding a dependency on tslint.

This configuration also ignores any eslintIgnore or eslintConfig blocks in package.json unless the EXTEND_ESLINT environment variable is set [3].

Proposal: Modify the project to use TSLint through react-scripts-ts

• Using tslint will prevent programmer error when trying to make lint changes in tslint.json
• react-scripts-ts uses tslint directly instead of eslint with TypeScript support [4]
• react-scripts-ts doesn't require environment variables to apply TSLint config
• Supports aligning frontend development practices with kubeflow/pipelines which uses tslint

Tasks for implementation:

• Switch react-scripts to react-scripts-ts in package.json to pick up tslint.json
• Remove eslint configuration lines from package.json
• Format the codebase using tslint

Caveats

There are a lot of existing files in the codebase that don’t match the rules declared in tslint.json, so adding tslint will break the webpack compile until the lint errors are fixed. If we do this in conjunction with using adding prettier for code styles (#141), we can do both as one large-scale reformatting and reduce the merge overhead.

References

[1] https://github.com/facebook/create-react-app/blob/6d6dfa9ba5a6042ab5690fe2a38b3ee54e644c9f/packages/react-scripts/config/webpack.config.js#L349

[2] https://github.com/facebook/create-react-app/blob/6d6dfa9ba5a6042ab5690fe2a38b3ee54e644c9f/packages/eslint-config-react-app/index.js#L57

[3]
https://github.com/facebook/create-react-app/blob/6d6dfa9ba5a6042ab5690fe2a38b3ee54e644c9f/packages/react-scripts/config/webpack.config.js#L359

[4] https://github.com/jpavon/react-scripts-ts/blob/d80cda5b72d17aeb217b23b331d9eba225319b3c/config/paths.js#L62

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[kwasi]

/area front-end

[jtfogarty]

/priority p2