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

Is it changeable flycheck temporary files path? #1446

Closed
johnny-mh opened this issue Apr 10, 2018 · 11 comments · Fixed by #1704
Closed

Is it changeable flycheck temporary files path? #1446

johnny-mh opened this issue Apr 10, 2018 · 11 comments · Fixed by #1704
Labels
kind: upstream status: has pr Problem identified, has PR

Comments

@johnny-mh
Copy link

johnny-mh commented Apr 10, 2018

Karam test runner watching my source directory to rerun test suite.

flycheck makes temporary files to same directory with source code.

so the test runner run tests every key typing and finally process die.

I had use option for prefix but it doesn't help.

because the test runner watching entire source directory.

can i change path of flycheck temporary files?

@fmdkdd
Copy link
Member

fmdkdd commented Apr 10, 2018

You are using the typescript-tslint checker, right?

There is no option to change the directory of temporary files that are created using source-inplace, because the point of source-inplace is to create files /in the same directory/ as the file being checked. Some checkers (tslint in this case) rely on this behavior in order to correctly check files that have dependencies.

In this case, source-inplace was added to fix #1420.

You can try changing the source-inplace to source in the definition of typescript-tslint and see if that works for you (this will create temporary files under /tmp instead).

Otherwise, I would recommend to instruct your test runner to ignore some files (most file watchers should have this option for various reasons).

cc @Simplify

@johnny-mh
Copy link
Author

johnny-mh commented Apr 11, 2018

@fmdkdd Thank you. I just changed flycheck.el file that source-inplace to source on tslint checker. it works nice. but is that OK? Do i need change that code every flycheck package updates?

@fmdkdd
Copy link
Member

fmdkdd commented Apr 11, 2018

Do i need change that code every flycheck package updates?

If you want to make it permanent, then you could copy and paste the whole checker definition into your own Emacs configuration. It would override the built-in Flycheck typescript-tslint.

But maybe we have to rethink this source-inplace afterall. I'm interested in what @Simplify thinks.

@Simplify
Copy link
Member

I'll prefer source-inplace to stay because of integration with typescript-tide and growing number of rules in tslint that depend on tsconfig.

@everedifice Temporally files that Flycheck generates are in format flycheck_<original-filename> so you can exclude **/flycheck(*) in your karma config i guess?

@Deewiant
Copy link

Chiming in with another case where the current status quo is painful. I'm using Tide together with Flycheck on an application that was bootstrapped with create-react-app, i.e. https://github.com/wmonk/create-react-app-typescript. While running the file watcher (yarn start or npm run start) and making code changes, eventually the whole server dies due to an error like (paths scrubbed):

FatalError:
Invalid source file: ..../src/flycheck_Something.tsx. Ensure that the files supplied to lint have a .ts, .tsx, .d.ts, .js or .jsx extension.

    at new FatalError (..../node_modules/tslint/lib/error.js:27:28)
    at Linter.getSourceFile (..../node_modules/tslint/lib/linter.js:222:23)
    at Linter.lint (..../node_modules/tslint/lib/linter.js:96:31)
    at ..../node_modules/fork-ts-checker-webpack-plugin/lib/IncrementalChecker.js:142:30
    at WorkSet.forEach (..../node_modules/fork-ts-checker-webpack-plugin/lib/WorkSet.js:17:13)
    at IncrementalChecker.getLints (..../node_modules/fork-ts-checker-webpack-plugin/lib/IncrementalChecker.js:139:17)
    at run (..../node_modules/fork-ts-checker-webpack-plugin/lib/service.js:15:29)
    at process.<anonymous> (..../node_modules/fork-ts-checker-webpack-plugin/lib/service.js:38:5)
    at process.emit (events.js:180:13)
    at emit (internal/child_process.js:783:12)

I think this is a race condition somewhere: the watcher notices the file, but fails to open it because it's already gone, and this error propagates all the way up.

The annoyance with this is that the one start script is not just for linting's sake, but also triggers tsc and serves the app locally, so the workflow is generally interrupted at the point when one's web browser says that the server is down, which is quite jarring.

As far as I can tell, there's no way of handling this in tool configuration currently. Excludes are only picked up from tslint.json and if I add a flycheck_* exclude there then Flycheck's own linting is also disabled.

I'm not really sure where and how this should be fixed, though. I suppose TSLint being capable of handling stdin (palantir/tslint#1590) would be the best.

In the meanwhile I think that source-original (i.e. telling Flycheck to not use temporary files here) would be a suitable workaround for me, but it really sucks that such a minor change entails copying the whole checker definition as @fmdkdd suggested above.

@fmdkdd
Copy link
Member

fmdkdd commented Apr 13, 2018

such a minor change entails copying the whole checker definition as @fmdkdd suggested above

You can target the source-inplace more directly by modifying the :command property directly. Like so:

(setcar (memq 'source-inplace (flycheck-checker-get 'typescript-tslint 'command))
        'source-original)

If you do that after the checker is defined, it should work.

But I'm more interested in finding a proper solution that would work for everyone out of the box.

@cpitclaudel
Copy link
Member

As far as I can tell, there's no way of handling this in tool configuration currently. Excludes are only picked up from tslint.json and if I add a flycheck_* exclude there then Flycheck's own linting is also disabled.

Can you expand on this part? Is the issue that tslint doesn't report errors for excluded files, even if you pass them explicitly?

@Deewiant
Copy link

Deewiant commented Apr 14, 2018

@fmdkdd If that does work (EDIT: confirmed, it does) then it's certainly more robust, thanks.

@cpitclaudel Yes, that's how it works at least as of TSLint 5.9.1. As is easily demonstrated:

$ echo 'const x = 1' > x.ts
$ echo '{"rules": {"semicolon":"true"}}' > tslint.json
$ tslint x.ts

ERROR: x.ts[1, 12]: Missing semicolon

$ echo '{"rules": {"semicolon":"true"}, "linterOptions": {"exclude": ["x.ts"]}}' > tslint.json
$ tslint x.ts
[no output]

@cpitclaudel
Copy link
Member

Thanks. It looks like there's progress on the tslint side, so hopefully we'll soon have a much better solution (palantir/tslint#3816)

@Simplify
Copy link
Member

create-react-app-typescript is definitely a problem. It uses webpack when you run yarn start, but webpack config is buried somewhere in react-scripts-ts, so adding flycheck_* to the list of ignored files is almost impossible.

As mention in GH-1472, I'll like to revert source-inplace. Those who need source-inplace can modify command directly.

jpb added a commit to unbounce/iidy that referenced this issue Nov 7, 2018
I was unfortunately not able to solve this any other way
flycheck/flycheck#1446
tavisrudd pushed a commit to unbounce/iidy that referenced this issue Nov 7, 2018
I was unfortunately not able to solve this any other way
flycheck/flycheck#1446
cpitclaudel added a commit that referenced this issue Mar 20, 2020
Closes GH-1446 and GH-1472, as eslint supports --stdin, so there are no temp
file problems.  Also closes GH-1585: with tslint being deprecated, we do want to
override it with eslint.
@cpitclaudel cpitclaudel added the status: has pr Problem identified, has PR label Mar 20, 2020
cpitclaudel added a commit that referenced this issue Mar 22, 2020
Closes GH-1446 and GH-1472, as eslint supports --stdin, so there are no temp
file problems.  Also closes GH-1585: with tslint being deprecated, we do want to
override it with eslint.
cpitclaudel added a commit that referenced this issue May 4, 2020
Closes GH-1446 and GH-1472, as eslint supports --stdin, so there are no temp
file problems.  Also closes GH-1585: with tslint being deprecated, we do want to
override it with eslint.
cpitclaudel added a commit that referenced this issue May 12, 2020
Closes GH-1446 and GH-1472, as eslint supports --stdin, so there are no temp
file problems.  Also closes GH-1585: with tslint being deprecated, we do want to
override it with eslint.
cpitclaudel added a commit that referenced this issue May 12, 2020
Closes GH-1446 and GH-1472, as eslint supports --stdin, so there are no temp
file problems.  Also closes GH-1585: with tslint being deprecated, we do want to
override it with eslint.
@JulienPalard
Copy link

Just hit the issue. I was working on pylint (but if it was not a linter, it would have be the same), and flycheck created a flycheck_similar.py file near similar.py file, which pylint spotted as a plugin to load, but failed loading it (as "both" use the same optparse things, there's a conflict which raises an exception).

bbatsov pushed a commit that referenced this issue Feb 5, 2024
Closes GH-1446 and GH-1472, as eslint supports --stdin, so there are no temp
file problems.  Also closes GH-1585: with tslint being deprecated, we do want to
override it with eslint.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: upstream status: has pr Problem identified, has PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants