8000 [2.4.0][Watch] Creating or renaming files throws errors · Issue #1091 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]
Skip to content

[2.4.0][Watch] Creating or renaming files throws errors #1091

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.

Sign up for GitHub

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

Jump to bottom
Closed
fubar opened this issue Oct 15, 2019 · 15 comments · Fixed by #1106
Closed

[2.4.0][Watch] Creating or renaming files throws errors #1091

fubar opened this issue Oct 15, 2019 · 15 comments · Fixed by #1106
Labels
bug Something isn't working locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: typescript-estree Issues related to @typescript-eslint/typescript-estree

Comments

@fubar
Copy link
fubar commented Oct 15, 2019 •
edited
Loading

Original issue tracked in #864

This issue still occurs in Intellij on the latest version 2.4.0 (https://github.com/typescript-eslint/typescript-eslint/releases/tag/v2.4.0) (which includes PR #973). The original reproduction steps apply: Create a new file or rename an existing one and Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser. is thrown:

Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: foo.ts
The file must be included in at least one of the projects provided.

Versions

package version
@typescript-eslint/parser 2.4.0
TypeScript 3.6.4
ESLint 6.5.1
node 11.10.1
@fubar fubar added package: parser Issues related to @typescript-eslint/parser triage Waiting for team members to take a look labels Oct 15, 2019
@bradzacher
Copy link
Member

renaming a file is a KP - renaming a file wasn't handled, because I assumed that it would trigger similar file system events to creating a file. #1081

Creating a new file has a minor known problem which is a race condition. Below is a rough representation of the events that are triggered:

  • The IDE creates the file
    • The IDE opens the file
      • The IDE's ESLint plugin attempts a file lint (1)
    • The file system triggers a directory change event
      • TypeScript delays the 8000 update by 200ms to batch events and reduce thrashing.
      • Typescript updates the program (2)

If (1) happens before (2), then you will receive the parsing error, because typescript does not yet know about the new file.
If (2) happens before (1), then there will be no parsing error.
In my testing I found that it was up in the air as to which one happened first, and depended on how fast the IDE was at opening the file, and how responsive the filesystem events decided to be.

If the error occurs, then it should be able to be resolved by simply closing and reopening the file.
Does this work for you?

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party package: typescript-estree Issues related to @typescript-eslint/typescript-estree and removed triage Waiting for team members to take a look package: parser Issues related to @typescript-eslint/parser labels Oct 15, 2019
@fubar
Copy link
Author
fubar commented Oct 15, 2019

Thanks for the explanation! Closing and reopening the file does not solve it, the error remains. Not sure if it helps but I don't recall this being an issue on @typescript-eslint/parser v1.4.2.

@bradzacher
Copy link
Member

No, this was not a problem in 1.4.2 - we made changes to improve the perf in 2.0.0, which had an adverse effect on the IDE experience.

We're in a weird spot as a parser, namely because of these three things:

  1. We operate at the project level, but ESLint operates at the file level.
  2. We are stateful (because typescript is stateful), but ESLint is not.
  3. We want to do some work asynchronously, but ESLint is synchronous.

(1) In v1, we noticed that most users suffered small-huge performance issues because they specified files outside of the project defined by their tsconfig(s). This caused perf problems because typescript would create a new program root and parse the file (and all of its dependencies - i.e. double typechecking many files). To help fix this, in 2.0.0 we started throwing errors for files that weren't included in the project.

(2) This change was great for CLI users, however it caused a problem with the IDE use case. ESLint has no concept of a long running, stateful lint session, and just has an API for consumers to call when a file needs to be linted. Typescript is stateful, and has a saved snapshot of the filesystem at the time of the first file parse. Creating a new file, obviously means there is a new file that is not contained within that state. So in 2.4.0 we added file system watchers which attempt to tell typescript to update its representation when a new file is created.

(3) As noted however, this process is very much asynchronous, but eslint has no concept of asynchronous parsing, so there is a race condition.

I tested this solution in VSCode, with the eslint extension, and it all worked well (https://www.youtube.com/watch?v=0My26ejZJVk).
Other IDE plugins have different codebases which could be causing the problem (i.e. they could be caching results, etc), or there could be another problem. I'll have to look into it more.

We've got some ideas in the pipeline to attempt to fix this better, but it's a very large problem across many, many teams and pieces, so some of it is bandaids right now.

@bradzacher bradzacher added triage Waiting for team members to take a look bug Something isn't working and removed awaiting response Issues waiting for a reply from the OP or another party labels Oct 15, 2019
@fubar
Copy link
Author
fubar commented Oct 15, 2019

Again, thanks for the explanation! If I can help debug or test in any way, more than happy to.

@bradzacher bradzacher mentioned this issue Oct 16, 2019
Closed
@cvle cvle mentioned this issue Oct 16, 2019
Merged
@bradzacher bradzacher pinned this issue Oct 17, 2019
@bradzacher bradzacher changed the title Creating or renaming file throws "Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser." [2.4.0][Watch] Creating or renaming files throws errors Oct 17, 2019
@bradzacher bradzacher unpinned this issue Oct 17, 2019
@bradzacher bradzacher pinned this issue Oct 17, 2019
@bradzacher bradzacher mentioned this issue Oct 17, 2019
Closed
@bradzacher bradzacher removed the triage Waiting for team members to take a look label Oct 17, 2019
@bradzacher bradzacher mentioned this issue Oct 17, 2019
Closed
@zedryas
Copy link
zedryas commented Oct 17, 2019 •
edited
Loading

https://www.youtube.com/watch?v=0My26ejZJVk

I did try since i was using the advanced new file plugin for vscode to creates new file, to, as you do in the video, direct create a file (right click -> new file) - but didn't work either.

Any specific on the configuration you use in the project on the video? could you share the relevant config part? as for me it's absolutely non working even when closing reopenning the file.
The only solution is to close/reopen the editor or to reload it

@bradzacher
Copy link
Member
bradzacher commented Oct 17, 2019 •
edited
Loading

TL;DR - I relied upon my tests, but my tests didn't cover enough cases.

So I think I know what happened.

  1. I recorded that video with a set of code.
  2. I wrote a set of tests which tested code.
  3. I then found a bug in the code
  4. I fixed that bug, ensuring the tests passed.
  5. I then put up the PR, linking the video.

The problem is that I didn't test my change in vscode again.
My tests were passing, but they were too simple, and didn't represent an actual project. My tests tested one situation: where the modified files were in the root folder: include: ["src"], and I created a file in the src folder.

However, the change I made broke detecting changes in deeply nested folders, because my change from (4) incorrectly interpreted when typescript asks for a directory watcher with recursive: undefined as being the same as false (no recursion), when instead, it is supposed to mean infinite recursion depth.

This meant that in a real project which has config include: ["src"], a new file in src/foo did not get detected.

@bradzacher bradzacher mentioned this issue Oct 19, 2019
Merged
@Hotell Hotell mentioned this issue Oct 19, 2019
Closed
@bradzacher
Copy link
Member

I've just landed #1106, and it will be deployed to the canary tag imminently.
This change reworks the invalidation approach so that it doesn't need file watchers at all.

I'd appreciate if you could take a moment to test the canary tag on your codebases, and let me know if anything isn't working as expected for your real-world codebase.

@bradzacher bradzacher unpinned this issue Oct 19, 2019
@fubar
Copy link
Author
fubar commented Oct 21, 2019

@bradzacher this looks promising, thanks for taking the time to dig into it! Unfortunately it doesn't fix it for me in Intellij though. I just tried v2.5.0 (released a few hours ago and includes your fix) and the same error is shown when adding and renaming files.

In case it helps, the error shows up with about a 3 second delay. Disabling and enabling eslint generally takes a few seconds to take effect so not sure how relevant this is. I'm also occasionally seeing a "Can not get result from language service" error after re-enabling eslint, but this one resolves automatically after a second or two and I can't reliably reproduce it.

What can I do to give you more insight?

@bopfer
Copy link
bopfer commented Oct 21, 2019

I am also still seeing the parser error in VSCode with v2.5.0:

eslint-typescript-parse-error

Here are my parser options:

parserOptions: {
    project: "./tsconfig.eslint.json",
    tsconfigRootDir: __dirname,
  },

And my tsconfig.eslint.json file:

{
  "extends": "./tsconfig.json",
  "include": [
    "*.js",
    "src/**/*",
    "scripts/**/*"
  ]
}

I am also using a Yarn workspace if that is relevant. Let me know if there is anything else useful I can provide.

@bradzacher
Copy link
Member
bradzacher commented Oct 21, 2019 •
edited
Loading

I'm also occasionally seeing a "Can not get result from language service" error after re-enabling eslint

I don't know what this error message means. I haven't ever used IntelliJ, so I don't know the context in which it would show you this message.
This message isn't from our parser / plugin - we don't do anything with language services.

I have another fix in #1111 which might help this a bit more.
It was just merged - there will be a canary version in ~15 minutes.

@fiznool
Copy link
fiznool commented Oct 22, 2019

I have another fix in #1111 which might help this a bit more. It was just merged - there will be a canary version in ~15 minutes.

Just to note that I've tested with 2.5.1-alpha.0 (the current canary release) and I'm still seeing the same parser error in VSCode, as described elsewhere. For me, this is happening when I create a new file or move an existing file to a new folder.

@bradzacher
Copy link
Member

Could you please give me some more information about your environment @fiznool?
It's hard to investigate the problem without it.

What OS are you using?
What does your tsconfig(s) look like?
Are you using a monorepo?

@fiznool
Copy link
fiznool commented Oct 22, 2019

@bradzacher this is a very early stage project, and as such I can probably clone it into a public repo for you to take a look at. I will check and get back to you.

Otherwise, I’m using macOS Catalina with stable VSCode.

@vschoener
Copy link
vschoener commented Dec 2, 2019 •
edited
Loading

I have the same problem on last VSCode issue and Mac OS when creating a new file, this error occurs and I have to restart Vscode to make it update that the file is part of the project.

EDIT: Just seeing below but closing and reopening the file fix it.

Edit: In fact, closing reopening doesn't work everything

@bradzacher
Copy link
Member

Please raise a new issue and give as much info as possible.

@typescript-eslint typescript-eslint locked as resolved and limited conversation to collaborators Dec 2, 2019
@bradzacher bradzacher added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Apr 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Something isn't working locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: typescript-estree Issues related to @typescript-eslint/typescript-estree
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(typescript-estree): support long running lint without watch typescript-eslint/typescript-eslint
6 participants
@fiznool @bopfer @vschoener @fubar @zedryas @bradzacher
0