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

Option to use tslint in fix mode #67

Closed
pelotom opened this issue Nov 16, 2017 · 23 comments
Closed

Option to use tslint in fix mode #67

pelotom opened this issue Nov 16, 2017 · 23 comments

Comments

@pelotom
Copy link
Contributor

pelotom commented Nov 16, 2017

Is it possible to make tslint run with the --fix option?

@johnnyreilly
Copy link
Member

I think that's in 0.2.9 of fork-ts-checker-webpack-plugin. I believe @piotr-oles is presently battling to release it right now...

@pelotom
Copy link
Contributor Author

pelotom commented Nov 16, 2017

Great, can't wait!

@johnnyreilly
Copy link
Member

The 0.2.9 release has been slightly delayed for some fairly understandable reasons. If you're too impatient to wait for it to land on npm you could use this in the short-term: https://github.com/typestrong/fork-ts-checker-webpack-plugin

nb The repo above will be removed from GitHub once 0.2.9 is released to npm. This is just for those impatient people out there (well, me mostly 😉)

@pelotom
Copy link
Contributor Author

pelotom commented Nov 17, 2017

Thanks for the heads up!

@alecmev
Copy link

alecmev commented Nov 23, 2017

@johnnyreilly Mm, 0.2.9 is out, but I don't see anything in the code suggesting that fixing can be enabled. It's hardcoded to false right here.

@johnnyreilly
Copy link
Member

Maybe submit a PR then? It's not my project but PRs are taken!

@alecmev
Copy link

alecmev commented Nov 23, 2017

I think that's in 0.2.9 of fork-ts-checker-webpack-plugin

Why did you claim this then? ^

@pelotom
Copy link
Contributor Author

pelotom commented Nov 23, 2017

@jeremejevs dude, chill. He said "I think", which is not a "claim". He's not the maintainer and was just trying to be helpful with an educated guess. If you know where it's hardcoded then you're in a position to submit a PR, so maybe stop complaining and contribute.

@alecmev
Copy link

alecmev commented Nov 23, 2017

If I were complaining, it'd be justified, since this "I think" made me go on a wild goose chase, thinking that there must be some branch or a PR or a fork somewhere, with the --fix setting in it. Why would one say "I think X" without any reason to say that? But it wasn't even a complaint, it was a question.

@johnnyreilly
Copy link
Member

Much 💓 for having my back @pelotom

@jeremejevs - it's not my project I was clearly mistaken. But now you have an opportunity to make the world a slightly better place. Go to it! Submit a PR 👍

@piotr-oles
Copy link
Collaborator

Sorry I didn't reply. There is no such option in 0.2.9. I think we could add such option, PR's are welcome.
I would like to add this feature but I'm not sure when I will have enough free time to do this. So like I said - don't hesitate to add PR :)

@alecmev
Copy link

alecmev commented Nov 24, 2017

Looks like this isn't as simple as setting fix to true. Here's a relevant discussion: palantir/tslint#3188 Tried some quick potential fixes (like not passing oldProgram to createProgram), to no avail. Ultimately decided to not invest any extra time into this, because the only reason I wanted auto-fixing in the first place was Prettier, and the onchange watch approach is comparably clean, more flexible and working right now (mentioning this for future readers with similar needs).

Sorry for the passive-aggressive yesterday.

@johnnyreilly
Copy link
Member

No worries @jeremejevs. Thanks for reporting back - that's helpful

@Hoishin
Copy link

Hoishin commented May 9, 2018

Have there been any progress on this issue? Looks like the discussion @jeremejevs linked (palantir/tslint#3188) was solved, so it could be fixed now?

@alecmev
Copy link

alecmev commented May 9, 2018

The discussion I linked to isn't a blocker, it just has some useful information. This can be fixed (ha, fixed), but it just isn't a trivial problem, and doesn't bring enough benefit to be worth the time investment, at least for those who have looked into it so far.

@piotr-oles
Copy link
Collaborator

I close the issue because there was no PR since November 2017 and I think using tslint with some git hooks is a lot easier than messing with files during development.

@ajainarayanan
Copy link
Contributor

ajainarayanan commented Oct 23, 2018

@piotr-oles / @johnnyreilly I know this is very late but is this feature still considered? Would a PR with tslintAutoFix as option to ForkTsCheckerWebpackPlugin plugin be acceptable?

@johnnyreilly
Copy link
Member

I think using tslint with some git hooks is a lot easier than messing with files during development.

I kind of agree with @piotr-oles - it seems a little strange to have a type checker also changing your files. Doesn't seem like a good fit. Sorry

@ajainarayanan
Copy link
Contributor

@johnnyreilly I understand. Since this was going to an option it could by default be turned off and in cases where users need to enable it they could just enable it.

The reason I am trying for this solution is we right now have ForkTsCheckerWebpackPlugin to do both type check and linting check. If not I need to disable tslint check on ForkTsCheckerWebpackPlugin and manually configure tslint just for this reason.

One last try to request to have this as an option and turn it off by default. If not thanks for the response!

@johnnyreilly
Copy link
Member

johnnyreilly commented Oct 23, 2018

No worries!

Just so I understand you correctly, you have your code linted and fixed as you're working on a file?

Forgive the surprise, I've just never encountered that before!

Out of curiosity, do you have an idea how simple an implementation might be?

@ajainarayanan
Copy link
Contributor

@johnnyreilly I am not sure what would be the difference between linted and fixed but I presume lint is checking for lint errors and fix is fixing them.

Yes. I am using prettier that is run as tslint rule. prettier formats and provides formatting options that can be run through tslint --fix that will automatically be fixed. This helps us to lint code in the webpack build as and when the user saves the file.

This is also very common in vscode environments where when I type in the code and save the file the file gets automatically linted.

To give more context, the environment I am working on has a webapp that has a mix of both js and ts. For js we use prettier to be run as part of eslint and we have enabled autofix to fix linting errors as and when we save the file. For tslint we wanted to do similar thing and trying to see if I can do it via ForkTsCheckerWebpackPlugin

@johnnyreilly
Copy link
Member

Hmmm. Still feels a little strange to me!

Would you like to have a try and see how hard this is to implement? If it's super simple with minimal impact to the codebase then I could see a case for supporting this.

If it turns out to be complicated and not so simple then perhaps not.

@ajainarayanan
Copy link
Contributor

@johnnyreilly It believe it should be simple. Need to pass the flag from the plugin to the service and have the service check if its in the process.env. If not default to false.

It would be easier for me to create a pull request to see how big the impact is.

johnnyreilly pushed a commit that referenced this issue Nov 7, 2018
…lugin plugin (#174)

* Adds tslintAutoFix flag to ForkTsCheckerWebpackPlugin

* Updates Vue integration test based on IncrementalChecker class update

* Adds integration testing for tslintAutoFix flag

* Updates README file to add description about tslintAutoFix flag

* Removes unnecessary tslint checks

* Adds a check to verify formatted file contents

* Adds seperate tslint autofix json to avoid formatting files other than autofix files in integration test

* Bumps package.json and updates CHANGELOG file for tslintAutoFix option

* Updates README for tslintAutoFix with default value as false
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants