Language Service host API cleanup #1735
Conversation
…r node package consumers
… is never honored by the LS. The LS operate only on a fixed list of files initialized by the host's getScriptFileNames(), if a file is not in this list, it will not be queried from the host, rather considered a missing file. Thus the result of getDefaultLibFilename()is either in the list, or will be ignored.
Conflicts: src/services/services.ts
|
@csnover this change simplifies the host API, take a look and let me know if you see other inconsistencies. still to come is documentation for these APIs. |
| @@ -9,7 +9,7 @@ | |||
| /// <reference path='formatting\smartIndenter.ts' /> | |||
|
|
|||
| module ts { | |||
| export var servicesVersion = "0.4" | |||
| export var servicesVersion = "0.4.1" | |||
There was a problem hiding this comment.
Just noticed, put a semicolon.
There was a problem hiding this comment.
Bumping up the API version will require updating the check on the managed side as well
|
Looks good, once the minor changes I've suggested are addressed, 👍 |
|
It will be a few days for additional feedback from me. It’s up to you if you want to wait :) |
| * The functionality is not supported if the ts module is consumed outside of a node module. | ||
| */ | ||
| export function getDefaultLibraryFilePath(options: CompilerOptions): string { | ||
| if (typeof module !== "undefined" && module.exports) { |
There was a problem hiding this comment.
i'm not sure what this means, or why it is correct.
There was a problem hiding this comment.
+1. not sure what we are checking here and why?
There was a problem hiding this comment.
checking if we are in node. i am open to suggestions.. this is how we do it in sys anyways.
There was a problem hiding this comment.
If you just want to use __dirname then check typeof __dirname. This current line isn’t a good check for being in a Node.js environment. You can get the same invalid answer with a define(function (require, exports, module) {}) AMD wrapper or if someone just pollutes some globals. Probably the “best” check for knowing if you are in an environment that is intentionally trying to quack like Node.js is typeof process !== 'undefined' && process.versions && process.versions.node but in any case actually trying to detect the API you want to use is best.
There was a problem hiding this comment.
changed to check for __dirname
|
Good simplification. Did you check whether WScript will also give you the directory of the executing .js file? 👍 |
|
👍 |
Conflicts: src/services/services.ts
Conflicts: src/services/services.ts
…check the extension
…ail throw an exception
Language Service host API cleanup
Few cleanup changes to the host API also adding a helper function to get the default library path for node packages.