Skip to content
This repository was archived by the owner on Oct 16, 2020. It is now read-only.

Need to handle diagnostics without file field #239

Closed
tomv564 opened this issue May 7, 2017 · 7 comments · Fixed by #243
Closed

Need to handle diagnostics without file field #239

tomv564 opened this issue May 7, 2017 · 7 comments · Fixed by #243
Labels

Comments

@tomv564
Copy link
Contributor

tomv564 commented May 7, 2017

The language service is failing to return diagnostics and logging errors when compiler diagnostics are returned alongside file diagnostics.

These diagnostics (despite the typing of ts.Diagnostic) have no file field (typically just a code and a message).

To avoid exceptions handling those diagnostics, adding a tsDiagnostics.filter(f => !!f.file) is needed before calling convertTsDiagnostic in _publishDiagnostics

@felixfbecker
Copy link
Contributor

Could you report that as a bug to TypeScript as well? The interface doesn't mention that file is optional

@tomv564
Copy link
Contributor Author

tomv564 commented May 8, 2017

@felixfbecker
Copy link
Contributor

The declared type of the property is SourceFile, not SourceFile | undefined. TS' type system doesn't differentiate between present properties and undefined properties, if they had enabled strictNullChecks the code you linked would cause a compile error.

@felixfbecker
Copy link
Contributor

Filed microsoft/TypeScript#15666

@tomv564
Copy link
Contributor Author

tomv564 commented May 8, 2017

Ah, just as I threw microsoft/TypeScript#15667 on the pile.

@tomv564
Copy link
Contributor Author

tomv564 commented May 8, 2017

I closed mine. Surprised they are not running with strictNullChecks already - my expectation was to find an as Diagnostic somewhere :)

@felixfbecker
Copy link
Contributor

Yes, I am receiving so many cannot read x of undefined errors from within the TS codebase that could probably be all solved with that flag.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants