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

Task: Typescript errors disappear after document closed #47386

Closed
neoncom opened this issue Apr 7, 2018 · 21 comments · Fixed by #183271
Closed

Task: Typescript errors disappear after document closed #47386

neoncom opened this issue Apr 7, 2018 · 21 comments · Fixed by #183271
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders typescript Typescript support issues verified Verification succeeded

Comments

@neoncom
Copy link

neoncom commented Apr 7, 2018

  • VSCode Version: 1.22.1 (newest) (typescript 2.8.1)
  • OS Version: Windows 10 64bit

I expect typescript errors from all files to show until they have been fixed.

Instead, they disappear as soon as the document is closed again (e.g. by randomly clicking on the error list, which causes documents to open & close again)

Steps to Reproduce:

  1. Use this tasks.json (check if the project path fits you first)
{
    "version": "2.0.0",
    "command": "tsc",
    "isShellCommand": true,
    "args": [
        "-w",
        "-p",
        "."
    ],
    "showOutput": "silent",
    "isBackground": true,
    "problemMatcher": {
        "base": "$tsc-watch",
        // "owner": "typescript",
        //  "applyTo": "allDocuments"
    }
}
  1. Start the task
  2. Errors will show in error list (hopefully, if there are errors at all)
  3. Randomly click between errors of different files in error list. The files will open and close automatically (if they are not pinned). When a file closes, it's errors disappear!

What I have tried:

  • $tsc-watch matcher without any other options
  • owner property "typescript" or something artificial
  • applyTo property all possibilities
  • in settings: typescript.validate.enable === false

The "best" in my trial and error endevour I have got was that the errors just persisted, even when fixed, lol :)

My user settings (don't have workspace settings)

{
    "terminal.integrated.shell.windows": "C:\\Windows\\System32\\cmd.exe",
    "editor.wordWrap": "on",
    "explorer.confirmDelete": false,
    "editor.quickSuggestions": {
        "other": false,
        "comments": false,
        "strings": false
    },
    "editor.autoClosingBrackets": false,
    "explorer.confirmDragAndDrop": false,
    "editor.acceptSuggestionOnCommitCharacter": false,
    "window.zoomLevel": 0,
    "explorer.autoReveal": true,
    "editor.suggestOnTriggerCharacters": true,
    "editor.acceptSuggestionOnEnter": "on",
    "editor.parameterHints": true,
    "typescript.referencesCodeLens.enabled": false,
    // "typescript.validate.enable": false
}

Does this issue occur when all extensions are disabled?: Yes

@vscodebot vscodebot bot assigned mjbvz Apr 7, 2018
@vscodebot vscodebot bot added the typescript Typescript support issues label Apr 7, 2018
@dbaeumer
Copy link
Member

@mjbvz the reason for this is that the errors from the TS language server and the tsc -w task have the same owner (which they need, otherwise you see errors twice). Now when the file closes the TS extensions removes the errors from the file. I somehow remember that I had special code in that I fetched errors from files that have been through an open / close cycle until the file disappears.

The same problem happens for example without the tsc watch task. Have two ts files open where one depends on the other, have an error and close the one with the error. Observer the error list is empty.

At the end the TS language server should have a project wide compile.

For the tsc -w case I can look into restoring errors when files are closing.

@massimonewsuk
Copy link

We recently swapped from Flow to TypeScript.

In our previous workflow with Flow, the errors would remain when we opened/closed files.

However, with Typescript, none of the errors show by default (when all files are closed). For this reason, we use VSCode's Tasks -> Run Build Task, to run the detected tsconfig.watch. This is great because it shows all errors across the project without having to open files.

However, when we open a file then close it, the error disappears from the problems tab (same issue as described above).

We would love for there to be a configuration option to disable removing errors from problems tab until they are fixed. This would be a simple fix. An even cooler one would be for this behaviour to be automatic when using a typescript watcher.

@dbaeumer
Copy link
Member

dbaeumer commented May 7, 2018

@mjbvz in the past the typescript extension did keep all files that were open and reported errors in a list and rechecked them even if they were closed. This feature seems to got removed. Any reason for this?

@massimonewsuk
Copy link

massimonewsuk commented May 16, 2018

Our team moved from Flow to TypeScript recently and this is one thing they keep bringing up as a downside... @mjbvz any comments on this?

@peterstarling
Copy link

this looks like a regression to me, it didn't use to be like that

@dbaeumer
Copy link
Member

You can always run the tsc: watchtaks contributed by the TypeScript extension. This will compile the whole project and watch for file changes.

@massimonewsuk
Copy link

Yes but after you close a file, the errors disappear from the file browser bar and the IDE's problems summary

@dbaeumer
Copy link
Member

Agree. (see my command #47386 (comment))

@mjbvz mjbvz added this to the May 2018 milestone May 22, 2018
@mjbvz mjbvz added the bug Issue identified by VS Code Team member as probable bug label May 22, 2018
@mjbvz
Copy link
Contributor

mjbvz commented May 22, 2018

I’m checking in a change to keep the diagnostics around on close, but the problem de-duplication seems incorrect here. Here are the states:

Open workspace with no open files and start watch task

  • Should get errors for index.ts from problem matcher

Now open index.ts in vscode

  • Still should have errors from problem matcher
  • And now we should have errors from TS too.
  • These have same owner so only one instance of each problem should be reported by vs code in the problems panel

Close index.ts

  • Back to first state
  • We should start using the problems matcher errors again
  • But currently we go to having zero errors instead

@dbaeumer
Copy link
Member

dbaeumer commented May 23, 2018

This is not fully correct:

Now open index.ts in vscode

  • since the errors from the problem matcher and the TS server have the same owner id the errors of the problem matchers are basically replaced by the one from the TS Server

Close index.ts

  • TS server takes errors away.
  • But since the compiler in watch mode is not triggered again (no file change usually on close) no new errors are generated and the errors are lost.

I looked into re-applying the errors on document close however this is not so easy due to that fact that everything runs async. If I re-add them too early the TS server will take them away if I re-add them too late then the whole problem view flickers. So we need to have some sort of coordination here which very likely requires new API.

@mjbvz
Copy link
Contributor

mjbvz commented May 23, 2018

That makes sense for how the feature is implemented, but as an extension author I expected that VS Code would maintain both errors behind the scenes and then presents the de-duplicated error list to users. Is that possible? Would it break existing scenarios?

@dbaeumer
Copy link
Member

No, the problem with this is that de-duping would only work if both error sets are computed on the same state. However usually the compiler works on the save state on disk whereas the typescript server works on the in buffer state. Consider the case where there is one error from the compiler and then the user opens the file and adds two new lines at the top. Will be very hard to de-dupe these reliable. So the model for TS is more:

  • compiler owns problems on closed files
  • server owns problems on open files

The problem is to make transitions without flickering and showing errors twice. Am not sure if we can handle this generically in the core or whether we best involve the extension into this since that one might have more knowledge.

@mjbvz
Copy link
Contributor

mjbvz commented May 23, 2018

Thanks for the explanation. I'm closing this issue since we have a workaround on the ts side then

@mjbvz mjbvz closed this as completed May 23, 2018
@massimonewsuk
Copy link

massimonewsuk commented May 24, 2018

@mjbvz Could you ping us the GitHub issue which will result in this behaviour being fixed? (i.e. if it's in the TS repo or something). Just so we can have progress updates 😄

@dinofx
Copy link

dinofx commented Apr 14, 2020

Once the TS language server has provided problems, perhaps the matched problems could be temporarily suppressed, but not deduped (lost)?

@trusktr
Copy link

trusktr commented Nov 3, 2020

Seems this is still a problem nowadays.

Right now,

  • tsc --watch is reporting errors and on first run they appear in Problems pane.
  • If I open and close one of the files with errors, those errors are removed from Problems pane.
  • If I edit an unrelated file, which produces new output from tsc --watch, those errors never come back although they are re-outputted to stdout again.
  • The only way to get the errors back is to look at the terminal output to figure out which files have errors and then open those files.

This is fairly time consuming.

It would be great to show all (current and non-outdated) errors for the whole project in its current state, and have an option to disable that like some other people want (they want only errors for open files).

@ugultopu
Copy link
Contributor

@mjbvz

Thanks for the explanation. I'm closing this issue since we have a workaround on the ts side then

Just to be clear, the workaround for TypeScript is to run the tsc: watch task, which is not a task that is manually defined, but is available due to the presence of a tsconfig.json file in a project. Is that correct?

russelldavis added a commit to russelldavis/vscode that referenced this issue May 23, 2023
Fixes microsoft#47386

Logic was added in microsoft@df97bc3
to add a watcher's problems back to the problems view after closing a
file.

It doesn't work quite right though because of how `processLineInternal`
batches up changes, only sending them to `markerService` when it gets
called on subsequent lines with a different owner or resource.

Because of that, the current behavior is that when you close a file with
errors, the errors will disappear and only end up getting restored the
next time a different file being watched gets saved.

This PR fixes that (restoring the errors right after the file is closed)
by calling `forceDelivery()`.
@russelldavis
Copy link
Contributor

Note: this was partially fixed by df97bc3. Due to a bug, the current behavior is that when you close a file with problems, the problems will get restored, but only after the next time a different file being watched gets saved. I have a fix for that at #183172.

With that PR, problems from a watch task will get added back to the problems view right after closing a file.

russelldavis added a commit to russelldavis/vscode that referenced this issue May 23, 2023
Fixes microsoft#47386

This sets the `flushOnListenerRemove` option to true for the call to
Event.debounce. Without this, some `onMarkerChanged` events can get
dropped -- specifically, when an event gets fired more than 100ms after
first listening, in which case the 600ms cleanup setTimeout fires while
the 500ms debounce timeout is still pending.
russelldavis added a commit to russelldavis/vscode that referenced this issue May 23, 2023
Fixes microsoft#47386

This sets the `flushOnListenerRemove` option to true for the call to
Event.debounce. Without this, some `onMarkerChanged` events can get
dropped -- specifically, when an event gets fired more than 100ms after
first listening, in which case the 600ms cleanup setTimeout fires while
the 500ms debounce timeout is still pending.
russelldavis added a commit to russelldavis/vscode that referenced this issue Jun 9, 2023
Fixes microsoft#47386

This sets the `flushOnListenerRemove` option to true for the call to
Event.debounce. Without this, some `onMarkerChanged` events can get
dropped -- specifically, when an event gets fired more than 100ms after
first listening, in which case the 600ms cleanup setTimeout fires while
the 500ms debounce timeout is still pending.
russelldavis added a commit to russelldavis/vscode that referenced this issue Jul 18, 2023
Fixes microsoft#47386

This sets the `flushOnListenerRemove` option to true for the call to
Event.debounce. Without this, some `onMarkerChanged` events can get
dropped -- specifically, when an event gets fired more than 100ms after
first listening, in which case the 600ms cleanup setTimeout fires while
the 500ms debounce timeout is still pending.
russelldavis added a commit to russelldavis/vscode that referenced this issue Sep 30, 2023
Fixes microsoft#47386

This sets the `flushOnListenerRemove` option to true for the call to
Event.debounce. Without this, some `onMarkerChanged` events can get
dropped -- specifically, when an event gets fired more than 100ms after
first listening, in which case the 600ms cleanup setTimeout fires while
the 500ms debounce timeout is still pending.
russelldavis added a commit to russelldavis/vscode that referenced this issue Oct 2, 2023
Fixes microsoft#47386

This sets the `flushOnListenerRemove` option to true for the call to
Event.debounce. Without this, some `onMarkerChanged` events can get
dropped -- specifically, when an event gets fired more than 100ms after
first listening, in which case the 600ms cleanup setTimeout fires while
the 500ms debounce timeout is still pending.
russelldavis added a commit to russelldavis/vscode that referenced this issue Oct 3, 2023
Fixes microsoft#47386

This sets the `flushOnListenerRemove` option to true for the call to
Event.debounce. Without this, some `onMarkerChanged` events can get
dropped -- specifically, when an event gets fired more than 100ms after
first listening, in which case the 600ms cleanup setTimeout fires while
the 500ms debounce timeout is still pending.
russelldavis added a commit to russelldavis/vscode that referenced this issue Oct 7, 2023
Fixes microsoft#47386

This sets the `flushOnListenerRemove` option to true for the call to
Event.debounce. Without this, some `onMarkerChanged` events can get
dropped -- specifically, when an event gets fired more than 100ms after
first listening, in which case the 600ms cleanup setTimeout fires while
the 500ms debounce timeout is still pending.
russelldavis added a commit to russelldavis/vscode that referenced this issue Oct 12, 2023
Fixes microsoft#47386

This sets the `flushOnListenerRemove` option to true for the call to
Event.debounce. Without this, some `onMarkerChanged` events can get
dropped -- specifically, when an event gets fired more than 100ms after
first listening, in which case the 600ms cleanup setTimeout fires while
the 500ms debounce timeout is still pending.
@meganrogge meganrogge self-assigned this Nov 15, 2023
@meganrogge meganrogge modified the milestones: Backlog, November 2023 Nov 15, 2023
@mjbvz mjbvz modified the milestones: November 2023, December 2023 Nov 21, 2023
@starball5
Copy link

starball5 commented Nov 27, 2023

Does this have any relation to the typescript.tsserver.experimental.enableProjectDiagnostics setting?

meganrogge added a commit that referenced this issue Dec 19, 2023
…183271)

Fixes #47386

This sets the `flushOnListenerRemove` option to true for the call to
Event.debounce. Without this, some `onMarkerChanged` events can get
dropped -- specifically, when an event gets fired more than 100ms after
first listening, in which case the 600ms cleanup setTimeout fires while
the 500ms debounce timeout is still pending.

Co-authored-by: Megan Rogge <merogge@microsoft.com>
@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Dec 19, 2023
@alexr00 alexr00 added the verified Verification succeeded label Jan 25, 2024
@aiday-mar aiday-mar added this to the December / January 2024 milestone Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders typescript Typescript support issues verified Verification succeeded
Projects
None yet