[TypeScript] Request syntax diagnostics when "typescript.tsserver.useSeparateSyntaxServer": "forAllRequests"

[amcasey]

After a week or so of using partial mode full-time, my biggest complaint is that it doesn't flag obvious syntactic errors - particularly unbalanced braces. I can see why we'd want to avoid having partial mode show squiggles when it's enabled temporarily, during project load, but I think it might be nice to enable syntactic diagnostics when forAllRequests is set. Having said that, we almost certainly don't want semantic diagnostics, because there will be tons of unresolved symbols and anys.

This will probably require enabling a capability (semantic?) that allows getErr requests to be sent and, unless it is gated on a correspondingly updated TS version, a filter to drop semanticDiag and suggestionDiag events.

[amcasey]

FYI @mjbvz

[mjbvz]

Fixed on the VS Code side by 8b3d666

Closing now that upstream fix has been merged too

[mjbvz]

To verify:

1. In our repo, cd extensions then yarn add typescript@next (we still ship TS 4.3 by default since 4.4 with the fix is not yet stable)
2. yarn compile-web
3. yarn web to start web
4. In a TS file, try adding some syntax errors and confirm these are reported as errors

[alexr00]

Following those instructions got me this error:
"Activating extension 'vscode.typescript-language-features' failed: Invalid destructuring assignment target."

[mjbvz]

@amcasey I think something may have changed/regressed on the TypeScript side. I see geterr requests in web but don't see any events being generated

To test this out:

1. Checkout dev/mjbvz/bundle-44
2. yarn
3. yarn watch and then also yarn compile-web
4. Start browser with yarn web

Here's an example geterr request:

[Trace  - 05:13:34.887] <syntax> Sending request: geterr (191). Response expected: yes. Current queue length: 0
Arguments: {
    "delay": 0,
    "files": [
        "^memfs:/sample-folder/large.ts"
    ]
}

[amcasey]

@mjbvz Looking

[amcasey]

I'm seeing

Err 20    [13:43:25.417] Exception on executing command delayed processing of request 7:

    Illegal invocation

    TypeError: Illegal invocation
        at t.delay (http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3148256)
        at s.g.updateErrorCheck (http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3176374)
        at s.g.getDiagnostics (http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3201463)
        at http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3163087
        at t.executeAction (http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3148701)
        at t.startNew (http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3147490)
        at handlers.e.Map.e.getEntries.n.<computed> (http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3163056)
        at http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3216726
        at s.g.executeWithRequestId (http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3216550)
        at s.g.executeCommand (http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3216680)
        at s.g.onMessage (http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3217600)
        at http://localhost:8080/static/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js:15:3280217

Info 21   [13:43:25.417] Session does not support events: ignored event: {"seq":0,"type":"event","event":"requestCompleted","body":{"request_seq":7}}

[amcasey]

@mjbvz It works for me in code.bat, but not in yarn web. I had only ever validated the functionality in code.bat - had you seen it in the web version?

[mjbvz]

Yes I believe it used to work in the web as well

Is that error coming TS server? We did update from webpack 4 -> 5 since I tested this as well so could bundling be causing any issues?

[amcasey]

Yeah, it's from tsserver.web.js. The F12 tools keep freezing when I try to break into it - possibly because there are 3M characters on that line.

Edit: it eventually loaded, but I still don't understand what's going wrong.

[amcasey]

I've created microsoft/TypeScript#45242 to fix the exception, but the web server will still drop all events on the floor (not new). Can you please either file a feature request against TS or skip the call to getErr in this scenario?

[mjbvz]

Great find @amcasey!

I swear I saw this working but I think I was using a custom branch of TypeScript. We need to use getErr to get any diagnostics, correct?

[amcasey]

Most of the credit goes to @andrewbranch. 😄

Yes, you would need to use getErr for any diagnostics. If you really wanted to, I suppose you could request diagnostics synchronously (i.e. without events), but that might fail for other reasons.

[mjbvz]

Ah I thought microsoft/TypeScript#44859 also would cover web? Was that only for desktop partial mode? If so, I'll open a new issue for supporting syntax errors on web too

[amcasey]

It does cover web in the sense that the event is generated. Unfortunately, it turns out that web then drops that event on the floor, rather than sending it back to the editor. I agree that the scenario seems important, so please go ahead a file a new issue.

e.g. Here's the log entry I see when I run the web editor

Info 24   [17:06:07.547] Session does not support events: ignored event: {"seq":0,"type":"event","event":"syntaxDiag","body":{"file":"^memfs:/sample-folder/large.ts","diagnostics":[]}}

[DanielRosenwasser]

What was the reasoning behind not supporting events in the first place? Just not needing them?

[amcasey]

@DanielRosenwasser I misread - it does drop the event on the floor, but that's because !canUseEvents and not because it's a web server.

https://github.com/microsoft/TypeScript/blob/bfd5b2f7f09e95da708260c5b386a8987bd381f4/src/webServer/webServer.ts#L195

[mjbvz]

Upstream issue with fix for web microsoft/TypeScript#45313

[mjbvz]

Fixed by pulling in latest TS version

For verification, you should just be able to use yarn web now and check that introducing syntax errors in to a TS file generates an error