Skip to content

Commit 49ba408

Browse files
sheetalkamatsheetalkamat
committed
Handle scoped package names in typing installer
Fixes microsoft#32075
1 parent 664671c commit 49ba408

File tree

4 files changed

+114
-56
lines changed

4 files changed

+114
-56
lines changed

‎src/jsTyping/jsTyping.ts‎

Lines changed: 51 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -289,9 +289,8 @@ namespace ts.JsTyping {
289289

290290
}
291291

292-
export const enum PackageNameValidationResult {
292+
export const enum NameValidationResult {
293293
Ok,
294-
ScopedPackagesNotSupported,
295294
EmptyName,
296295
NameTooLong,
297296
NameStartsWithDot,
@@ -301,49 +300,77 @@ namespace ts.JsTyping {
301300

302301
const maxPackageNameLength = 214;
303302

303+
export interface ScopedPackageNameValidationResult {
304+
name: string;
305+
isScopeName: boolean;
306+
result: NameValidationResult;
307+
}
308+
export type PackageNameValidationResult = NameValidationResult | ScopedPackageNameValidationResult;
309+
304310
/**
305311
* Validates package name using rules defined at https://docs.npmjs.com/files/package.json
306312
*/
307313
export function validatePackageName(packageName: string): PackageNameValidationResult {
314+
return validatePackageNameWorker(packageName, /*supportScopedPackage*/ true);
315+
}
316+
317+
function validatePackageNameWorker(packageName: string, supportScopedPackage: false): NameValidationResult;
318+
function validatePackageNameWorker(packageName: string, supportScopedPackage: true): PackageNameValidationResult;
319+
function validatePackageNameWorker(packageName: string, supportScopedPackage: boolean): PackageNameValidationResult {
308320
if (!packageName) {
309-
return PackageNameValidationResult.EmptyName;
321+
return NameValidationResult.EmptyName;
310322
}
311323
if (packageName.length > maxPackageNameLength) {
312-
return PackageNameValidationResult.NameTooLong;
324+
return NameValidationResult.NameTooLong;
313325
}
314326
if (packageName.charCodeAt(0) === CharacterCodes.dot) {
315-
return PackageNameValidationResult.NameStartsWithDot;
327+
return NameValidationResult.NameStartsWithDot;
316328
}
317329
if (packageName.charCodeAt(0) === CharacterCodes._) {
318-
return PackageNameValidationResult.NameStartsWithUnderscore;
330+
return NameValidationResult.NameStartsWithUnderscore;
319331
}
320332
// check if name is scope package like: starts with @ and has one '/' in the middle
321333
// scoped packages are not currently supported
322-
// TODO: when support will be added we'll need to split and check both scope and package name
323-
if (/^@[^/]+\/[^/]+$/.test(packageName)) {
324-
return PackageNameValidationResult.ScopedPackagesNotSupported;
334+
if (supportScopedPackage) {
335+
const matches = /^@([^/]+)\/([^/]+)$/.exec(packageName);
336+
if (matches) {
337+
const scopeResult = validatePackageNameWorker(matches[1], /*supportScopedPackage*/ false);
338+
if (scopeResult !== NameValidationResult.Ok) {
339+
return { name: matches[1], isScopeName: true, result: scopeResult };
340+
}
341+
const packageResult = validatePackageNameWorker(matches[2], /*supportScopedPackage*/ false);
342+
if (packageResult !== NameValidationResult.Ok) {
343+
return { name: matches[2], isScopeName: false, result: packageResult };
344+
}
345+
return NameValidationResult.Ok;
346+
}
325347
}
326348
if (encodeURIComponent(packageName) !== packageName) {
327-
return PackageNameValidationResult.NameContainsNonURISafeCharacters;
349+
return NameValidationResult.NameContainsNonURISafeCharacters;
328350
}
329-
return PackageNameValidationResult.Ok;
351+
return NameValidationResult.Ok;
330352
}
331353

332354
export function renderPackageNameValidationFailure(result: PackageNameValidationResult, typing: string): string {
355+
return typeof result === "object" ?
356+
renderPackageNameValidationFailureWorker(typing, result.result, result.name, result.isScopeName) :
357+
renderPackageNameValidationFailureWorker(typing, result, typing, /*isScopeName*/ false);
358+
}
359+
360+
function renderPackageNameValidationFailureWorker(typing: string, result: NameValidationResult, name: string, isScopeName: boolean): string {
361+
const kind = isScopeName ? "Scope" : "Package";
333362
switch (result) {
334-
case PackageNameValidationResult.EmptyName:
335-
return `Package name '${typing}' cannot be empty`;
336-
case PackageNameValidationResult.NameTooLong:
337-
return `Package name '${typing}' should be less than ${maxPackageNameLength} characters`;
338-
case PackageNameValidationResult.NameStartsWithDot:
339-
return `Package name '${typing}' cannot start with '.'`;
340-
case PackageNameValidationResult.NameStartsWithUnderscore:
341-
return `Package name '${typing}' cannot start with '_'`;
342-
case PackageNameValidationResult.ScopedPackagesNotSupported:
343-
return `Package '${typing}' is scoped and currently is not supported`;
344-
case PackageNameValidationResult.NameContainsNonURISafeCharacters:
345-
return `Package name '${typing}' contains non URI safe characters`;
346-
case PackageNameValidationResult.Ok:
363+
case NameValidationResult.EmptyName:
364+
return `'${typing}':: ${kind} name '${name}' cannot be empty`;
365+
case NameValidationResult.NameTooLong:
366+
return `'${typing}':: ${kind} name '${name}' should be less than ${maxPackageNameLength} characters`;
367+
case NameValidationResult.NameStartsWithDot:
368+
return `'${typing}':: ${kind} name '${name}' cannot start with '.'`;
369+
case NameValidationResult.NameStartsWithUnderscore:
370+
return `'${typing}':: ${kind} name '${name}' cannot start with '_'`;
371+
case NameValidationResult.NameContainsNonURISafeCharacters:
372+
return `'${typing}':: ${kind} name '${name}' contains non URI safe characters`;
373+
case NameValidationResult.Ok:
347374
return Debug.fail(); // Shouldn't have called this.
348375
default:
349376
throw Debug.assertNever(result);

‎src/testRunner/unittests/tsserver/typingsInstaller.ts‎

Lines changed: 47 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
namespace ts.projectSystem {
22
import validatePackageName = JsTyping.validatePackageName;
3-
import PackageNameValidationResult = JsTyping.PackageNameValidationResult;
3+
import NameValidationResult = JsTyping.NameValidationResult;
44

55
interface InstallerParams {
66
globalTypingsCacheLocation?: string;
@@ -948,7 +948,8 @@ namespace ts.projectSystem {
948948
path: "/a/b/app.js",
949949
content: `
950950
import * as fs from "fs";
951-
import * as commander from "commander";`
951+
import * as commander from "commander";
952+
import * as component from "@ember/component";`
952953
};
953954
const cachePath = "/a/cache";
954955
const node = {
@@ -959,14 +960,19 @@ namespace ts.projectSystem {
959960
path: cachePath + "/node_modules/@types/commander/index.d.ts",
960961
content: "export let y: string"
961962
};
963+
const emberComponentDirectory = "ember__component";
964+
const emberComponent = {
965+
path: `${cachePath}/node_modules/@types/${emberComponentDirectory}/index.d.ts`,
966+
content: "export let x: number"
967+
};
962968
const host = createServerHost([file]);
963969
const installer = new (class extends Installer {
964970
constructor() {
965971
super(host, { globalTypingsCacheLocation: cachePath, typesRegistry: createTypesRegistry("node", "commander") });
966972
}
967973
installWorker(_requestId: number, _args: string[], _cwd: string, cb: TI.RequestCompletedAction) {
968-
const installedTypings = ["@types/node", "@types/commander"];
969-
const typingFiles = [node, commander];
974+
const installedTypings = ["@types/node", "@types/commander", `@types/${emberComponentDirectory}`];
975+
const typingFiles = [node, commander, emberComponent];
970976
executeCommand(this, host, installedTypings, typingFiles, cb);
971977
}
972978
})();
@@ -980,9 +986,10 @@ namespace ts.projectSystem {
980986

981987
assert.isTrue(host.fileExists(node.path), "typings for 'node' should be created");
982988
assert.isTrue(host.fileExists(commander.path), "typings for 'commander' should be created");
989+
assert.isTrue(host.fileExists(emberComponent.path), "typings for 'commander' should be created");
983990

984991
host.checkTimeoutQueueLengthAndRun(2);
985-
checkProjectActualFiles(service.inferredProjects[0], [file.path, node.path, commander.path]);
992+
checkProjectActualFiles(service.inferredProjects[0], [file.path, node.path, commander.path, emberComponent.path]);
986993
});
987994

988995
it("should redo resolution that resolved to '.js' file after typings are installed", () => {
@@ -1263,21 +1270,44 @@ namespace ts.projectSystem {
12631270
for (let i = 0; i < 8; i++) {
12641271
packageName += packageName;
12651272
}
1266-
assert.equal(validatePackageName(packageName), PackageNameValidationResult.NameTooLong);
1273+
assert.equal(validatePackageName(packageName), NameValidationResult.NameTooLong);
1274+
});
1275+
it("package name cannot start with dot", () => {
1276+
assert.equal(validatePackageName(".foo"), NameValidationResult.NameStartsWithDot);
1277+
});
1278+
it("package name cannot start with underscore", () => {
1279+
assert.equal(validatePackageName("_foo"), NameValidationResult.NameStartsWithUnderscore);
1280+
});
1281+
it("package non URI safe characters are not supported", () => {
1282+
assert.equal(validatePackageName(" scope "), NameValidationResult.NameContainsNonURISafeCharacters);
1283+
assert.equal(validatePackageName("; say ‘Hello from TypeScript!’ #"), NameValidationResult.NameContainsNonURISafeCharacters);
1284+
assert.equal(validatePackageName("a/b/c"), NameValidationResult.NameContainsNonURISafeCharacters);
1285+
});
1286+
it("scoped package name is supported", () => {
1287+
assert.equal(validatePackageName("@scope/bar"), NameValidationResult.Ok);
1288+
});
1289+
it("scoped name in scoped package name cannot start with dot", () => {
1290+
assert.deepEqual(validatePackageName("@.scope/bar"), { name: ".scope", isScopeName: true, result: NameValidationResult.NameStartsWithDot });
1291+
assert.deepEqual(validatePackageName("@.scope/.bar"), { name: ".scope", isScopeName: true, result: NameValidationResult.NameStartsWithDot });
1292+
});
1293+
it("scope name in scoped package name cannot start with underscore", () => {
1294+
assert.deepEqual(validatePackageName("@_scope/bar"), { name: "_scope", isScopeName: true, result: NameValidationResult.NameStartsWithUnderscore });
1295+
assert.deepEqual(validatePackageName("@_scope/_bar"), { name: "_scope", isScopeName: true, result: NameValidationResult.NameStartsWithUnderscore });
12671296
});
1268-
it("name cannot start with dot", () => {
1269-
assert.equal(validatePackageName(".foo"), PackageNameValidationResult.NameStartsWithDot);
1297+
it("scope name in scoped package name with non URI safe characters are not supported", () => {
1298+
assert.deepEqual(validatePackageName("@ scope /bar"), { name: " scope ", isScopeName: true, result: NameValidationResult.NameContainsNonURISafeCharacters });
1299+
assert.deepEqual(validatePackageName("@; say ‘Hello from TypeScript!’ #/bar"), { name: "; say ‘Hello from TypeScript!’ #", isScopeName: true, result: NameValidationResult.NameContainsNonURISafeCharacters });
1300+
assert.deepEqual(validatePackageName("@ scope / bar "), { name: " scope ", isScopeName: true, result: NameValidationResult.NameContainsNonURISafeCharacters });
12701301
});
1271-
it("name cannot start with underscore", () => {
1272-
assert.equal(validatePackageName("_foo"), PackageNameValidationResult.NameStartsWithUnderscore);
1302+
it("package name in scoped package name cannot start with dot", () => {
1303+
assert.deepEqual(validatePackageName("@scope/.bar"), { name: ".bar", isScopeName: false, result: NameValidationResult.NameStartsWithDot });
12731304
});
1274-
it("scoped packages not supported", () => {
1275-
assert.equal(validatePackageName("@scope/bar"), PackageNameValidationResult.ScopedPackagesNotSupported);
1305+
it("package name in scoped package name cannot start with underscore", () => {
1306+
assert.deepEqual(validatePackageName("@scope/_bar"), { name: "_bar", isScopeName: false, result: NameValidationResult.NameStartsWithUnderscore });
12761307
});
1277-
it("non URI safe characters are not supported", () => {
1278-
assert.equal(validatePackageName(" scope "), PackageNameValidationResult.NameContainsNonURISafeCharacters);
1279-
assert.equal(validatePackageName("; say ‘Hello from TypeScript!’ #"), PackageNameValidationResult.NameContainsNonURISafeCharacters);
1280-
assert.equal(validatePackageName("a/b/c"), PackageNameValidationResult.NameContainsNonURISafeCharacters);
1308+
it("package name in scoped package name with non URI safe characters are not supported", () => {
1309+
assert.deepEqual(validatePackageName("@scope/ bar "), { name: " bar ", isScopeName: false, result: NameValidationResult.NameContainsNonURISafeCharacters });
1310+
assert.deepEqual(validatePackageName("@scope/; say ‘Hello from TypeScript!’ #"), { name: "; say ‘Hello from TypeScript!’ #", isScopeName: false, result: NameValidationResult.NameContainsNonURISafeCharacters });
12811311
});
12821312
});
12831313

@@ -1309,7 +1339,7 @@ namespace ts.projectSystem {
13091339
projectService.openClientFile(f1.path);
13101340

13111341
installer.checkPendingCommands(/*expectedCount*/ 0);
1312-
assert.isTrue(messages.indexOf("Package name '; say ‘Hello from TypeScript!’ #' contains non URI safe characters") > 0, "should find package with invalid name");
1342+
assert.isTrue(messages.indexOf("'; say ‘Hello from TypeScript!’ #':: Package name '; say ‘Hello from TypeScript!’ #' contains non URI safe characters") > 0, "should find package with invalid name");
13131343
});
13141344
});
13151345

‎src/tsserver/server.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ namespace ts.server {
248248
isKnownTypesPackageName(name: string): boolean {
249249
// We want to avoid looking this up in the registry as that is expensive. So first check that it's actually an NPM package.
250250
const validationResult = JsTyping.validatePackageName(name);
251-
if (validationResult !== JsTyping.PackageNameValidationResult.Ok) {
251+
if (validationResult !== JsTyping.NameValidationResult.Ok) {
252252
return false;
253253
}
254254

‎src/typingsInstallerCore/typingsInstaller.ts‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -268,27 +268,28 @@ namespace ts.server.typingsInstaller {
268268
}
269269

270270
private filterTypings(typingsToInstall: ReadonlyArray<string>): ReadonlyArray<string> {
271-
return typingsToInstall.filter(typing => {
272-
if (this.missingTypingsSet.get(typing)) {
273-
if (this.log.isEnabled()) this.log.writeLine(`'${typing}' is in missingTypingsSet - skipping...`);
274-
return false;
271+
return mapDefined(typingsToInstall, typing => {
272+
const typingKey = mangleScopedPackageName(typing);
273+
if (this.missingTypingsSet.get(typingKey)) {
274+
if (this.log.isEnabled()) this.log.writeLine(`'${typing}':: '${typingKey}' is in missingTypingsSet - skipping...`);
275+
return undefined;
275276
}
276277
const validationResult = JsTyping.validatePackageName(typing);
277-
if (validationResult !== JsTyping.PackageNameValidationResult.Ok) {
278+
if (validationResult !== JsTyping.NameValidationResult.Ok) {
278279
// add typing name to missing set so we won't process it again
279-
this.missingTypingsSet.set(typing, true);
280+
this.missingTypingsSet.set(typingKey, true);
280281
if (this.log.isEnabled()) this.log.writeLine(JsTyping.renderPackageNameValidationFailure(validationResult, typing));
281-
return false;
282+
return undefined;
282283
}
283-
if (!this.typesRegistry.has(typing)) {
284-
if (this.log.isEnabled()) this.log.writeLine(`Entry for package '${typing}' does not exist in local types registry - skipping...`);
285-
return false;
284+
if (!this.typesRegistry.has(typingKey)) {
285+
if (this.log.isEnabled()) this.log.writeLine(`'${typing}':: Entry for package '${typingKey}' does not exist in local types registry - skipping...`);
286+
return undefined;
286287
}
287-
if (this.packageNameToTypingLocation.get(typing) && JsTyping.isTypingUpToDate(this.packageNameToTypingLocation.get(typing)!, this.typesRegistry.get(typing)!)) {
288-
if (this.log.isEnabled()) this.log.writeLine(`'${typing}' already has an up-to-date typing - skipping...`);
289-
return false;
288+
if (this.packageNameToTypingLocation.get(typingKey) && JsTyping.isTypingUpToDate(this.packageNameToTypingLocation.get(typingKey)!, this.typesRegistry.get(typingKey)!)) {
289+
if (this.log.isEnabled()) this.log.writeLine(`'${typing}':: '${typingKey}' already has an up-to-date typing - skipping...`);
290+
return undefined;
290291
}
291-
return true;
292+
return typingKey;
292293
});
293294
}
294295

0 commit comments

Comments
 (0)