Send Script Kind based on languageId for js and ts files

[mjbvz]

http://stackoverflow.com/questions/42139118/is-error-checking-in-vs-code-exclusively-tied-to-the-file-extension

bug
When using custom file associations for js/ts files, we do not always open the files against TypeScript with the correct language mode.

Fix
Always send scriptKind to Typescript based on the languageId of the file

[mention-bot]

@mjbvz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @dbaeumer and @egamma to be potential reviewers.

[dgpratt]

@mjbvz SO would prefer I not "thank you" in the comments over there, so I'll thank you here :). I really appreciate you looking at this and knocking it out so quickly.

[mjbvz]

Thanks. Looking into this issue also revealed a few other inconsistent behaviors around custom file extensions that I'm going to investigate. Please let me know if you run into any other problems with this scenario and I'll take a look.

[dbaeumer]

@mjbvz there is a deeper underlying reason why we only do this for files without extension (typically script files). The tsserver can not resolve filenames other than with ts or js extension. If users for example have a file a.custom which has an import onto b and the filename of b is b.custom the TS server will issue an error on the import since it tries to find b.ts or b.d.ts or b.js (at least when I last tested it). This will IMO be very confusing. So before releasing this please check if the tsserver has fixed this. Otherwise you will get a new set of issues you can't fix :-)

[dgpratt]

@dbaeumer @mjbvz Even if that's still the case, I think this change will work for my scenario. As I imagine is typical, we have a gulp-based process that transpiles .es6 files into co-located .js files. And even if it doesn't work for some other scenarios, we're just trading one set of weird errors for another. In other words, I think this change may not be perfect, but it seems it would make things better or at least no worse.

But as I think on it further, if the tsserver only resolves file names with a .js or .ts extension, how does it work for .jsx or .tsx files?

[mjbvz]

@dbaeumer Thank you for the explanation. That explains why the code was so specifically scoped.

Comparing VSCode 1.9 to the new behavior with this change, I do not see how this change is a regression however. For instance, with a javascript project:

// test.es6
export const abc = 123

//index.js
import { abc  } from './test'

In VSCode 1.9, we do not generate any errors. The type of abc is any however because TypeScript cannot resolve the file. That makes sense here

If we're working with TypeScript file instead (index.ts instead of index.js) then we do generate an error in VSCode 1.9 about not being able to locate the module test.

---

After this change the errors are unchanged but now in the JavaScript case, we have more consistent behavior. Could you please explain your specific concerns a bit more or provide an example of where the behavior will be worse / more confusing?

Also, here's the TS issue regarding custom file extensions: microsoft/TypeScript#10939 Seems like they've decided against supporting them for now

[dbaeumer]

The biggest problem I have are not only the errors (should have been more precise in my first comment) it is more about how we best explain this to users. So far we said: JS and TS smartness is only supported in .js, .ts, d.ts, .jsx and .tsx files. With one exception: files without an extension that have the JS mode (and thay was fishy too and we only did it for the script kind without extensions).

Now one can open a .es6 file select a symbol and do Find references and will not find all matches (more confusing it will find the matches in the .es6 file open but not any in files that are referencing that file). Or imports are not resolved. Workspace symbols will not work either. As said I am not a fan of a behavior that is hard to explain to users.

[mjbvz]

@dbaeumer I agree that the behavior using custom file extensions is confusing, but I still struggling to understand how this change makes it worse. In VSCode 1.9, Find References and workspace symbols can already be triggered in .es6 files and they already return those incomplete results. From what I can tell, TS defaults to assuming that the .es6 file is typescript code.

After this change, I see mostly the same behavior except that we now explicitly tell TypeScript that the file is a JavaScript file

[dbaeumer]

@mjbvz looks like I already made it worse a while ago then. At the beginning I tried to only provide TS / JS smartness for file extension the tsserver is aware of. Exactly to avoid the confusion and the bugs that are coming from it.