Skip to content

Commit 7c22ad6

Browse files
author
Andy Hanson
committed
Respond to code review
1 parent ee9f892 commit 7c22ad6

File tree

16 files changed

+73
-73
lines changed

16 files changed

+73
-73
lines changed

‎src/compiler/moduleNameResolver.ts‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,18 +1048,27 @@ namespace ts {
10481048
const mangledScopedPackageSeparator = "__";
10491049

10501050
/** For a scoped package, we must look in `@types/foo__bar` instead of `@types/@foo/bar`. */
1051-
function mangleScopedPackage(moduleName: string, state: ModuleResolutionState): string {
1052-
if (startsWith(moduleName, "@")) {
1053-
const replaceSlash = moduleName.replace(ts.directorySeparator, mangledScopedPackageSeparator);
1054-
if (replaceSlash !== moduleName) {
1055-
const mangled = replaceSlash.slice(1); // Take off the "@"
1056-
if (state.traceEnabled) {
1057-
trace(state.host, Diagnostics.Scoped_package_detected_looking_in_0, mangled);
1058-
}
1059-
return mangled;
1051+
function mangleScopedPackage(packageName: string, state: ModuleResolutionState): string {
1052+
const mangled = getMangledNameForScopedPackage(packageName);
1053+
if (state.traceEnabled && mangled !== packageName) {
1054+
trace(state.host, Diagnostics.Scoped_package_detected_looking_in_0, mangled);
1055+
}
1056+
return mangled;
1057+
}
1058+
1059+
/* @internal */
1060+
export function getTypesPackageName(packageName: string): string {
1061+
return `@types/${getMangledNameForScopedPackage(packageName)}`;
1062+
}
1063+
1064+
function getMangledNameForScopedPackage(packageName: string): string {
1065+
if (startsWith(packageName, "@")) {
1066+
const replaceSlash = packageName.replace(ts.directorySeparator, mangledScopedPackageSeparator);
1067+
if (replaceSlash !== packageName) {
1068+
return replaceSlash.slice(1); // Take off the "@"
10601069
}
10611070
}
1062-
return moduleName;
1071+
return packageName;
10631072
}
10641073

10651074
/* @internal */

‎src/harness/harnessLanguageService.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,8 @@ namespace Harness.LanguageService {
183183

184184
/// Native adapter
185185
class NativeLanguageServiceHost extends LanguageServiceAdapterHost implements ts.LanguageServiceHost {
186-
tryGetTypesRegistry(): ts.Map<void> | undefined {
187-
if (this.typesRegistry === undefined) {
188-
ts.Debug.fail("fourslash test should set types registry.");
189-
}
190-
return this.typesRegistry;
186+
isKnownTypesPackageName(name: string): boolean {
187+
return this.typesRegistry && this.typesRegistry.has(name);
191188
}
192189
installPackage = ts.notImplemented;
193190

‎src/harness/unittests/languageService.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ export function Component(x: Config): any;`
2121
// to write an alias to a module's default export was referrenced across files and had no default export
2222
it("should be able to create a language service which can respond to deinition requests without throwing", () => {
2323
const languageService = ts.createLanguageService({
24-
tryGetTypesRegistry: notImplemented,
25-
installPackage: notImplemented,
2624
getCompilationSettings() {
2725
return {};
2826
},

‎src/harness/unittests/session.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ namespace ts.server {
372372
};
373373
const command = "test";
374374

375-
session.output(body, command, /*reqSeq*/ 0, /*success*/ true);
375+
session.output(body, command, /*reqSeq*/ 0);
376376

377377
expect(lastSent).to.deep.equal({
378378
seq: 0,
@@ -475,7 +475,7 @@ namespace ts.server {
475475
};
476476
const command = "test";
477477

478-
session.output(body, command, /*reqSeq*/ 0, /*success*/ true);
478+
session.output(body, command, /*reqSeq*/ 0);
479479

480480
expect(session.lastSent).to.deep.equal({
481481
seq: 0,
@@ -549,7 +549,7 @@ namespace ts.server {
549549
return;
550550
}
551551
if (response) {
552-
this.output(response, msg.command, msg.seq, /*success*/ true);
552+
this.output(response, msg.command, msg.seq);
553553
}
554554
}
555555

‎src/harness/unittests/tsserverProjectSystem.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ namespace ts.projectSystem {
7070

7171
protected postExecActions: PostExecAction[] = [];
7272

73-
tryGetTypesRegistry = notImplemented;
73+
isKnownTypesPackageName = notImplemented;
7474
installPackage = notImplemented;
7575

7676
executePendingCommands() {

‎src/server/client.ts‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -540,15 +540,7 @@ namespace ts.server {
540540
return response.body.map(entry => this.convertCodeActions(entry, file));
541541
}
542542

543-
applyCodeActionCommand(file: string, command: CodeActionCommand): PromiseLike<ApplyCodeActionCommandResult> {
544-
const args: protocol.ApplyCodeActionCommandRequestArgs = { file, command };
545-
546-
const request = this.processRequest<protocol.ApplyCodeActionCommandRequest>(CommandNames.ApplyCodeActionCommand, args);
547-
// TODO: how can we possibly get it synchronously here? But is SessionClient test-only?
548-
const response = this.processResponse<protocol.ApplyCodeActionCommandResponse>(request);
549-
550-
return PromiseImpl.resolved({ successMessage: response.message });
551-
}
543+
applyCodeActionCommand = notImplemented;
552544

553545
private createFileLocationOrRangeRequestArgs(positionOrRange: number | TextRange, fileName: string): protocol.FileLocationOrRangeRequestArgs {
554546
return typeof positionOrRange === "number"

‎src/server/project.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,8 @@ namespace ts.server {
245245
/*@internal*/
246246
protected abstract getProjectRootPath(): Path | undefined;
247247

248-
tryGetTypesRegistry(): Map<void> | undefined {
249-
return this.typingsCache.tryGetTypesRegistry();
248+
isKnownTypesPackageName(name: string): boolean {
249+
return this.typingsCache.isKnownTypesPackageName(name);
250250
}
251251
installPackage(options: InstallPackageOptions): PromiseLike<ApplyCodeActionCommandResult> {
252252
return this.typingsCache.installPackage({ ...options, projectRootPath: this.getProjectRootPath() });

‎src/server/protocol.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1561,6 +1561,7 @@ namespace ts.server.protocol {
15611561
description: string;
15621562
/** Text changes to apply to each file as part of the code action */
15631563
changes: FileCodeEdits[];
1564+
/** A command is an opaque object that should be passed to `ApplyCodeActionCommandRequestArgs` without modification. */
15641565
commands?: {}[];
15651566
}
15661567

‎src/server/server.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,8 @@ namespace ts.server {
250250
private activeRequestCount = 0;
251251
private requestQueue: QueuedOperation[] = [];
252252
private requestMap = createMap<QueuedOperation>(); // Maps operation ID to newest requestQueue entry with that ID
253+
/** We will lazily request the types registry on the first call to `isKnownTypesPackageName` and store it in `typesRegistryCache`. */
254+
private requestedRegistry: boolean;
253255
private typesRegistryCache: Map<void> | undefined;
254256

255257
// This number is essentially arbitrary. Processing more than one typings request
@@ -279,13 +281,20 @@ namespace ts.server {
279281
}
280282
}
281283

282-
tryGetTypesRegistry(): Map<void> | undefined {
283-
if (this.typesRegistryCache) {
284-
return this.typesRegistryCache;
284+
isKnownTypesPackageName(name: string): boolean {
285+
// We want to avoid looking this up in the registry as that is expensive. So first check that it's actually an NPM package.
286+
const validationResult = JsTyping.validatePackageName(name);
287+
if (validationResult !== JsTyping.PackageNameValidationResult.Ok) {
288+
return undefined;
285289
}
286290

291+
if (this.requestedRegistry) {
292+
return !!this.typesRegistryCache && this.typesRegistryCache.has(name);
293+
}
294+
295+
this.requestedRegistry = true;
287296
this.send({ kind: "typesRegistry" });
288-
return undefined;
297+
return false;
289298
}
290299

291300
installPackage(options: InstallPackageOptionsWithProjectRootPath): PromiseLike<ApplyCodeActionCommandResult> {

‎src/server/session.ts‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,12 @@ namespace ts.server {
411411
this.send(ev);
412412
}
413413

414-
public output(info: {} | undefined, cmdName: string, reqSeq: number, success: boolean, message?: string) {
414+
// For backwards-compatibility only.
415+
public output(info: any, cmdName: string, reqSeq?: number, errorMsg?: string): void {
416+
this.doOutput(info, cmdName, reqSeq, /*success*/ !errorMsg, errorMsg);
417+
}
418+
419+
private doOutput(info: {} | undefined, cmdName: string, reqSeq: number, success: boolean, message?: string): void {
415420
const res: protocol.Response = {
416421
seq: 0,
417422
type: "response",
@@ -1299,7 +1304,7 @@ namespace ts.server {
12991304
this.changeSeq++;
13001305
// make sure no changes happen before this one is finished
13011306
if (project.reloadScript(file, tempFileName)) {
1302-
this.output(undefined, CommandNames.Reload, reqSeq, /*success*/ true);
1307+
this.doOutput(/*info*/ undefined, CommandNames.Reload, reqSeq, /*success*/ true);
13031308
}
13041309
}
13051310

@@ -1539,7 +1544,7 @@ namespace ts.server {
15391544

15401545
private applyCodeActionCommand(commandName: string, requestSeq: number, args: protocol.ApplyCodeActionCommandRequestArgs): void {
15411546
const { file, project } = this.getFileAndProject(args);
1542-
const output = (success: boolean, message: string) => this.output({}, commandName, requestSeq, success, message);
1547+
const output = (success: boolean, message: string) => this.doOutput({}, commandName, requestSeq, success, message);
15431548
const command = args.command as CodeActionCommand; // They should be sending back the command we sent them.
15441549
project.getLanguageService().applyCodeActionCommand(file, command).then(
15451550
({ successMessage }) => { output(/*success*/ true, successMessage); },
@@ -1845,7 +1850,7 @@ namespace ts.server {
18451850
},
18461851
[CommandNames.Configure]: (request: protocol.ConfigureRequest) => {
18471852
this.projectService.setHostConfiguration(request.arguments);
1848-
this.output(undefined, CommandNames.Configure, request.seq, /*success*/ true);
1853+
this.doOutput(/*info*/ undefined, CommandNames.Configure, request.seq, /*success*/ true);
18491854
return this.notRequired();
18501855
},
18511856
[CommandNames.Reload]: (request: protocol.ReloadRequest) => {
@@ -1966,7 +1971,7 @@ namespace ts.server {
19661971
}
19671972
else {
19681973
this.logger.msg(`Unrecognized JSON command: ${JSON.stringify(request)}`, Msg.Err);
1969-
this.output(undefined, CommandNames.Unknown, request.seq, /*success*/ false, `Unrecognized JSON command: ${request.command}`);
1974+
this.doOutput(/*info*/ undefined, CommandNames.Unknown, request.seq, /*success*/ false, `Unrecognized JSON command: ${request.command}`);
19701975
return { responseRequired: false };
19711976
}
19721977
}
@@ -1997,21 +2002,21 @@ namespace ts.server {
19972002
}
19982003

19992004
if (response) {
2000-
this.output(response, request.command, request.seq, /*success*/ true);
2005+
this.doOutput(response, request.command, request.seq, /*success*/ true);
20012006
}
20022007
else if (responseRequired) {
2003-
this.output(undefined, request.command, request.seq, /*success*/ false, "No content available.");
2008+
this.doOutput(/*info*/ undefined, request.command, request.seq, /*success*/ false, "No content available.");
20042009
}
20052010
}
20062011
catch (err) {
20072012
if (err instanceof OperationCanceledException) {
20082013
// Handle cancellation exceptions
2009-
this.output({ canceled: true }, request.command, request.seq, /*success*/ true);
2014+
this.doOutput({ canceled: true }, request.command, request.seq, /*success*/ true);
20102015
return;
20112016
}
20122017
this.logError(err, message);
2013-
this.output(
2014-
undefined,
2018+
this.doOutput(
2019+
/*info*/ undefined,
20152020
request ? request.command : CommandNames.Unknown,
20162021
request ? request.seq : 0,
20172022
/*success*/ false,

0 commit comments

Comments
 (0)