-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
fix: support long running "watch" lint sessions #973
Conversation
This comment has been minimized.
This comment has been minimized.
I really like this approach... but as you mentioned, you're hitting the same issue that I initially did, where the process won't exit because it's holding onto the file watchers. Sounds like the cleanup function would be great if we can get it. |
I tried to do something tricky, but it didn't seem to work:
When you call the watchDirectory callback, typescript schedules an the update for 250ms later (so it doesn't trigger multiple updates for a "save all files" cycle from an IDE). Because the parser isn't asynchronous, we can't wait for this time... grr I did some research into handles in NodeJS. I don't see a way to create "weak" handles (i.e. handles which won't block the process). So I guess that either we get eslint to better support stateful parsers, or we create our own CLI wrapper. |
Another option is to have a virtual program host (fork-ts-checker-webpack-plugin pull request) or a language service host (ts-node) that would use actual file content provided by eslint (which also can give better support for virtual TypeScript files, like in Vue) and when file is linted again just update it. Though it still may require some changes in eslint since it would need to have all files for the first lint, but maybe there are workarounds. |
Interesting. I'll investigate this more tonight!
We do do that already. We treat eslint as the source of truth for file contents - to the point that we don't even attach file system watchers for any of the js/ts files being linted! The problem is that when in watch mode, new files can be introduced. These files are do not exist in the ts program that's already been constructed. So we need a way to tell when we need to tell typescript to update the program. This is where watching the directories (and watching the tsconfigs) comes in:
I did think about an approach that involves us manually checking an unknown file against the provided tsconfigs to determine which programs to recalculate, but there are two problems with this:
|
# Conflicts: # packages/typescript-estree/package.json
Codecov Report
@@ Coverage Diff @@
## master #973 +/- ##
==========================================
- Coverage 94.18% 94.01% -0.17%
==========================================
Files 115 115
Lines 5022 5064 +42
Branches 1406 1416 +10
==========================================
+ Hits 4730 4761 +31
- Misses 165 175 +10
- Partials 127 128 +1
|
cc @JamesHenry and @uniqueiniquity Thanks to the great suggestion by @ark120202 - I've gotten this working using This is ready for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this!
I’m wondering what the difference between this approach and the workaround mentioned here: #864 (comment) |
Good call out @skovhus, I've added an additional comment there: #864 (comment) |
Fixes #864
This PR adds support for watching directories and configuration files so that we can force typescript to recalculate the program if new files are added.
In this PR I also setup typescript project references for the repo.
This was because I was jumping back and forth between packages to test the linting and watching, and I was annoyed at having to ensure I'd built everything in the dep tree.
There is one very, very minor problem with this that I don't know how to solve right now.
In an IDE, there is a race between when the file is opened and the eslint plugin lints it, and when the ts program is updated.
This means that if the editor is particularly quick to open the file (<250ms), then the initial lint of the empty file will throw the "file not in project" parsing error.
However I figured out this doesn't last long. If you start typing, then the plugin will re-lint the file (which now exists in the ts program), and the parser error will disappear. Or you can close and reopen the file.
Video showing it off working in VSCode: https://youtu.be/0My26ejZJVk