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

Linting inconsistencies #102

Closed
nareshbhatia opened this issue Jul 6, 2017 · 8 comments
Closed

Linting inconsistencies #102

nareshbhatia opened this issue Jul 6, 2017 · 8 comments

Comments

@nareshbhatia
Copy link

Generated a fresh project using create-react-app my-app --scripts-version=react-scripts-ts:

  • The generated README.md contains references to ESLint instead of TSLint (I would have thought that Remove mentions of Eslint #6 would have fixed this).
  • Changing tslint.json does not seem to do anything. For example, I intentionally messed up the indentation in App.tsx and I don't see any errors/warnings in the browser console. Then I tightened the spec on indentation to "indent": [ true, "spaces", 4 ], still not errors.

Am I missing something?

@nareshbhatia
Copy link
Author

Can someone please confirm whether they are seeing these issues?

Thanks in advance.

@wmonk
Copy link
Owner

wmonk commented Jul 12, 2017

Is this related to #103?

@nareshbhatia
Copy link
Author

I don't think so. #103 says that lint reports only one error at a time. In my case no errors are reported even if I mess up the indentation completely!

In addition, my bullet 1 says that the generated README file has references to ESLint instead of TSLint.

@nareshbhatia
Copy link
Author

Just found out that tslint is reacting to all other issues but indentation. So the rule "indent": [ true, "spaces", 4 ] is simply saying that indentation should be 4 spaces. It does not seem to prevent badly indented code like this:

class App extends React.Component<{}, {}> {
render() {
return (
<div className="App">
<div className="App-header">
<img src={logo} className="App-logo" alt="logo" />
<h2>Welcome to React</h2>
</div>
<p className="App-intro">
To get started, edit <code>src/App.tsx</code> and save to reload.
</p>
</div>
);
}
}

Seems like a tslint issue to me!!! I should read up on it.

So the only issue remaining here is my bullet 1 in the initial description.

@wmonk
Copy link
Owner

wmonk commented Jul 13, 2017

With regards to your first bullet, I will gladly accept a PR to change all eslint to tslint! 👍

@nareshbhatia
Copy link
Author

@wmonk, I thought that #6 fixed that issue. There was a PR related to that one which was merged in. Is that not the case?

@nareshbhatia
Copy link
Author

@wmonk, ah, looking at the PR, README.md was not touched. I will try to make a new PR.

@nareshbhatia
Copy link
Author

I started looking for the README.md file that needs to be changed. Tracked it down to this file. I notice that it is nicely tracking the upstream. If I make changes to it, we will break that tracking. I don't think it is worth the trouble as long as we understand what is going on under the covers. Closing this issue.

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

No branches or pull requests

2 participants