Skip to content

Commit 798232c

Browse files
jramsayjramsay
committed
Fixing unit tests, lint errors & addressing CR feedback
1 parent 511b3cd commit 798232c

3 files changed

Lines changed: 55 additions & 54 deletions

File tree

‎src/harness/unittests/typingsInstaller.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44

55
namespace ts.projectSystem {
66
describe("typings installer", () => {
7-
it("configured projects (tsd installed) 1", () => {
7+
it("configured projects (typings installed) 1", () => {
8+
debugger;
89
const file1 = {
910
path: "/a/b/app.js",
1011
content: ""
@@ -31,7 +32,7 @@ namespace ts.projectSystem {
3132
};
3233

3334
const jquery = {
34-
path: "/a/data/typings/jquery/jquery.d.ts",
35+
path: "/a/data/node_modules/@types/jquery/index.d.ts",
3536
content: "declare const $: { x: number }"
3637
};
3738

@@ -44,18 +45,18 @@ namespace ts.projectSystem {
4445
const p = projectService.configuredProjects[0];
4546
checkProjectActualFiles(p, [file1.path]);
4647

47-
assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "tsd.json")));
48+
assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "package.json")));
4849

4950
installer.runPostInstallActions(t => {
5051
assert.deepEqual(t, ["jquery"]);
5152
host.createFileOrFolder(jquery, /*createParentDirectory*/ true);
52-
return ["jquery/jquery.d.ts"];
53+
return ["@types/jquery"];
5354
});
5455
checkNumberOfProjects(projectService, { configuredProjects: 1 });
5556
checkProjectActualFiles(p, [file1.path, jquery.path]);
5657
});
5758

58-
it("inferred project (tsd installed)", () => {
59+
it("inferred project (typings installed)", () => {
5960
const file1 = {
6061
path: "/a/b/app.js",
6162
content: ""
@@ -71,7 +72,7 @@ namespace ts.projectSystem {
7172
};
7273

7374
const jquery = {
74-
path: "/a/data/typings/jquery/jquery.d.ts",
75+
path: "/a/data/node_modules/@types/jquery/index.d.ts",
7576
content: "declare const $: { x: number }"
7677
};
7778
const host = createServerHost([file1, packageJson]);
@@ -84,12 +85,12 @@ namespace ts.projectSystem {
8485
const p = projectService.inferredProjects[0];
8586
checkProjectActualFiles(p, [file1.path]);
8687

87-
assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "tsd.json")));
88+
assert(host.fileExists(combinePaths(installer.globalTypingsCacheLocation, "package.json")));
8889

8990
installer.runPostInstallActions(t => {
9091
assert.deepEqual(t, ["jquery"]);
9192
host.createFileOrFolder(jquery, /*createParentDirectory*/ true);
92-
return ["jquery/jquery.d.ts"];
93+
return ["@types/jquery"];
9394
});
9495
checkNumberOfProjects(projectService, { inferredProjects: 1 });
9596
checkProjectActualFiles(p, [file1.path, jquery.path]);
@@ -166,7 +167,7 @@ namespace ts.projectSystem {
166167
enqueueIsCalled = true;
167168
super.enqueueInstallTypingsRequest(project, typingOptions);
168169
}
169-
runTsd(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void {
170+
runInstall(cachePath: string, typingsToInstall: string[], postInstallAction: (installedTypings: string[]) => void): void {
170171
assert.deepEqual(typingsToInstall, ["node"]);
171172
runInstallIsCalled = true;
172173
super.runInstall(cachePath, typingsToInstall, postInstallAction);

‎src/server/typingsInstaller/nodeTypingsInstaller.ts‎

Lines changed: 43 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -93,59 +93,59 @@ namespace ts.server.typingsInstaller {
9393
const id = this.installRunCount;
9494
this.installRunCount++;
9595
let execInstallCmdCount = 0;
96-
let filteredTypings: string[] = [];
96+
const filteredTypings: string[] = [];
9797
for (const typing of typingsToInstall) {
9898
const command = `npm view @types/${typing} --silent name`;
99-
if (this.log.isEnabled()) {
100-
this.log.writeLine(`Running npm view @types ${id}, command '${command}'.`);
101-
}
102-
this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => {
99+
this.execAsync("npm view", command, cachePath, id, (err, stdout, stderr) => {
100+
if (stdout) {
101+
filteredTypings.push(typing);
102+
}
103103
execInstallCmdCount++;
104-
if (this.log.isEnabled()) {
105-
this.log.writeLine(`npm install @types ${id} stdout: ${stdout}`);
106-
this.log.writeLine(`npm install @types ${id} stderr: ${stderr}`);
104+
if (execInstallCmdCount === typingsToInstall.length) {
105+
installFilteredTypings(this, filteredTypings);
107106
}
108-
if (stdout !== "") {
109-
filteredTypings.push(typing);
107+
});
108+
}
109+
110+
function installFilteredTypings(self: NodeTypingsInstaller, filteredTypings: string[]) {
111+
const command = `npm install ${filteredTypings.map(t => "@types/" + t).join(" ")} --save-dev`;
112+
self.execAsync("npm install", command, cachePath, id, (err, stdout, stderr) => {
113+
if (stdout) {
114+
reportInstalledTypings(self);
110115
}
111-
if (execInstallCmdCount >= typingsToInstall.length) {
112-
const command = `npm install ${filteredTypings.map(t => "@types/" + t).join(" ")} --save-dev`;
113-
if (this.log.isEnabled()) {
114-
this.log.writeLine(`Running npm install @types ${id}, command '${command}'. cache path '${cachePath}'`);
116+
});
117+
}
118+
119+
function reportInstalledTypings(self: NodeTypingsInstaller) {
120+
const command = "npm ls -json";
121+
self.execAsync("npm ls", command, cachePath, id, (err, stdout, stderr) => {
122+
let installedTypings: string[];
123+
try {
124+
const response = JSON.parse(stdout);
125+
if (response.dependencies) {
126+
installedTypings = getOwnKeys(response.dependencies);
115127
}
116-
this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => {
117-
if (this.log.isEnabled()) {
118-
this.log.writeLine(`npm install @types ${id} stdout: ${stdout}`);
119-
this.log.writeLine(`npm install @types ${id} stderr: ${stderr}`);
120-
}
121-
if (stdout === "") {
122-
return;
123-
}
124-
const command = "npm ls -json";
125-
this.exec(command, { cwd: cachePath }, (err, stdout, stderr) => {
126-
if (this.log.isEnabled()) {
127-
this.log.writeLine(`npm ls -json ${id} stdout: ${stdout}`);
128-
this.log.writeLine(`npm ls -json ${id} stderr: ${stderr}`);
129-
}
130-
const installedTypings: string[] = [];
131-
try {
132-
const response = JSON.parse(stdout);
133-
if (response.dependencies) {
134-
for (const typing in response.dependencies) {
135-
installedTypings.push(typing);
136-
}
137-
}
138-
}
139-
catch (e) {
140-
this.log.writeLine(`Error parsing installed @types dependencies. Error details: ${e.message}`);
141-
}
142-
postInstallAction(installedTypings);
143-
});
144-
});
145128
}
129+
catch (e) {
130+
self.log.writeLine(`Error parsing installed @types dependencies. Error details: ${e.message}`);
131+
}
132+
postInstallAction(installedTypings || []);
146133
});
147134
}
148135
}
136+
137+
private execAsync(prefix: string, command: string, cwd: string, requestId: number, cb: (err: Error, stdout: string, stderr: string) => void) {
138+
if (this.log.isEnabled()) {
139+
this.log.writeLine(`#${requestId} running command '${command}'.`);
140+
}
141+
this.exec(command, { cwd }, (err, stdout, stderr) => {
142+
if (this.log.isEnabled()) {
143+
this.log.writeLine(`${prefix} #${requestId} stdout: ${stdout}`);
144+
this.log.writeLine(`${prefix} #${requestId} stderr: ${stderr}`);
145+
}
146+
cb(err, stdout, stderr);
147+
});
148+
}
149149
}
150150

151151
function findArgument(argumentName: string) {

‎src/server/typingsInstaller/typingsInstaller.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ namespace ts.server.typingsInstaller {
135135
if (!packageName) {
136136
continue;
137137
}
138-
var typingFile = typingToFileName(cacheLocation, packageName, this.installTypingHost);
138+
const typingFile = typingToFileName(cacheLocation, packageName, this.installTypingHost);
139139
if (!typingFile) {
140140
continue;
141141
}
@@ -198,7 +198,7 @@ namespace ts.server.typingsInstaller {
198198
continue;
199199
}
200200
installedPackages[packageName] = true;
201-
var typingFile = typingToFileName(cachePath, packageName, this.installTypingHost);
201+
const typingFile = typingToFileName(cachePath, packageName, this.installTypingHost);
202202
if (!typingFile) {
203203
continue;
204204
}

0 commit comments

Comments
 (0)