Skip to content

Commit 1c7a29e

Browse files
mhegazymhegazy
authored
Port fixes from master into release-2.5 (#18361)
* Test: parsing of two-line @typedef jsdoc * Fix forEachChild's visit of JSDocTypedefTag Also remove JSDocTypeLiteral.jsdocTypeTag, which made no sense since it was only useful when storing information for its parent `@typedef` tag. * Update baselines * Inline variable to aid control flow * Improved caching scheme for anonymous types * Fix signature help * Optimize caching of type literals * Accept new baselines * Fix linting errors * Use canonicalized forms when comparing signatures * Minor changes * Document ThrottledOperations.schedule * Limit the number of unanswered typings installer requests If we send them all at once, we (apparently) hit a buffer limit in the node IPC channel and both TS Server and the typings installer become unresponsive. * Fix lint issues * Add explanatory comment
1 parent c476a08 commit 1c7a29e

16 files changed

+263
-245
lines changed

‎src/compiler/checker.ts‎

Lines changed: 145 additions & 182 deletions
Large diffs are not rendered by default.

‎src/compiler/parser.ts‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -438,8 +438,10 @@ namespace ts {
438438
visitNode(cbNode, (<JSDocTypedefTag>node).typeExpression);
439439
}
440440
case SyntaxKind.JSDocTypeLiteral:
441-
for (const tag of (node as JSDocTypeLiteral).jsDocPropertyTags) {
442-
visitNode(cbNode, tag);
441+
if ((node as JSDocTypeLiteral).jsDocPropertyTags) {
442+
for (const tag of (node as JSDocTypeLiteral).jsDocPropertyTags) {
443+
visitNode(cbNode, tag);
444+
}
443445
}
444446
return;
445447
case SyntaxKind.PartiallyEmittedExpression:
@@ -6667,19 +6669,18 @@ namespace ts {
66676669
if (!typeExpression || isObjectOrObjectArrayTypeReference(typeExpression.type)) {
66686670
let child: JSDocTypeTag | JSDocPropertyTag | false;
66696671
let jsdocTypeLiteral: JSDocTypeLiteral;
6670-
let alreadyHasTypeTag = false;
6672+
let childTypeTag: JSDocTypeTag;
66716673
const start = scanner.getStartPos();
66726674
while (child = tryParse(() => parseChildParameterOrPropertyTag(PropertyLikeParse.Property))) {
66736675
if (!jsdocTypeLiteral) {
66746676
jsdocTypeLiteral = <JSDocTypeLiteral>createNode(SyntaxKind.JSDocTypeLiteral, start);
66756677
}
66766678
if (child.kind === SyntaxKind.JSDocTypeTag) {
6677-
if (alreadyHasTypeTag) {
6679+
if (childTypeTag) {
66786680
break;
66796681
}
66806682
else {
6681-
jsdocTypeLiteral.jsDocTypeTag = child;
6682-
alreadyHasTypeTag = true;
6683+
childTypeTag = child;
66836684
}
66846685
}
66856686
else {
@@ -6693,7 +6694,9 @@ namespace ts {
66936694
if (typeExpression && typeExpression.type.kind === SyntaxKind.ArrayType) {
66946695
jsdocTypeLiteral.isArrayType = true;
66956696
}
6696-
typedefTag.typeExpression = finishNode(jsdocTypeLiteral);
6697+
typedefTag.typeExpression = childTypeTag && !isObjectOrObjectArrayTypeReference(childTypeTag.typeExpression.type) ?
6698+
childTypeTag.typeExpression :
6699+
finishNode(jsdocTypeLiteral);
66976700
}
66986701
}
66996702

‎src/compiler/types.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2150,7 +2150,6 @@ namespace ts {
21502150
export interface JSDocTypeLiteral extends JSDocType {
21512151
kind: SyntaxKind.JSDocTypeLiteral;
21522152
jsDocPropertyTags?: ReadonlyArray<JSDocPropertyLikeTag>;
2153-
jsDocTypeTag?: JSDocTypeTag;
21542153
/** If true, then this type literal represents an *array* of its type. */
21552154
isArrayType?: boolean;
21562155
}
@@ -3296,13 +3295,12 @@ namespace ts {
32963295
}
32973296

32983297
/* @internal */
3299-
export interface MappedType extends ObjectType {
3298+
export interface MappedType extends AnonymousType {
33003299
declaration: MappedTypeNode;
33013300
typeParameter?: TypeParameter;
33023301
constraintType?: Type;
33033302
templateType?: Type;
33043303
modifiersType?: Type;
3305-
mapper?: TypeMapper; // Instantiation mapper
33063304
}
33073305

33083306
export interface EvolvingArrayType extends ObjectType {
@@ -3412,6 +3410,8 @@ namespace ts {
34123410
/* @internal */
34133411
erasedSignatureCache?: Signature; // Erased version of signature (deferred)
34143412
/* @internal */
3413+
canonicalSignatureCache?: Signature; // Canonical version of signature (deferred)
3414+
/* @internal */
34153415
isolatedSignatureType?: ObjectType; // A manufactured type that just contains the signature for purposes of signature comparison
34163416
/* @internal */
34173417
typePredicate?: TypePredicate;
@@ -3433,8 +3433,6 @@ namespace ts {
34333433
/* @internal */
34343434
export interface TypeMapper {
34353435
(t: TypeParameter): Type;
3436-
mappedTypes?: TypeParameter[]; // Types mapped by this mapper
3437-
instantiations?: Type[]; // Cache of instantiations created using this type mapper.
34383436
}
34393437

34403438
export const enum InferencePriority {

‎src/server/server.ts‎

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -235,24 +235,39 @@ namespace ts.server {
235235
return `${d.getHours()}:${d.getMinutes()}:${d.getSeconds()}.${d.getMilliseconds()}`;
236236
}
237237

238+
interface QueuedOperation {
239+
operationId: string;
240+
operation: () => void;
241+
}
242+
238243
class NodeTypingsInstaller implements ITypingsInstaller {
239244
private installer: NodeChildProcess;
240245
private installerPidReported = false;
241246
private socket: NodeSocket;
242247
private projectService: ProjectService;
243-
private throttledOperations: ThrottledOperations;
244248
private eventSender: EventSender;
249+
private activeRequestCount = 0;
250+
private requestQueue: QueuedOperation[] = [];
251+
private requestMap = createMap<QueuedOperation>(); // Maps operation ID to newest requestQueue entry with that ID
252+
253+
// This number is essentially arbitrary. Processing more than one typings request
254+
// at a time makes sense, but having too many in the pipe results in a hang
255+
// (see https://github.com/nodejs/node/issues/7657).
256+
// It would be preferable to base our limit on the amount of space left in the
257+
// buffer, but we have yet to find a way to retrieve that value.
258+
private static readonly maxActiveRequestCount = 10;
259+
private static readonly requestDelayMillis = 100;
260+
245261

246262
constructor(
247263
private readonly telemetryEnabled: boolean,
248264
private readonly logger: server.Logger,
249-
host: ServerHost,
265+
private readonly host: ServerHost,
250266
eventPort: number,
251267
readonly globalTypingsCacheLocation: string,
252268
readonly typingSafeListLocation: string,
253269
private readonly npmLocation: string | undefined,
254270
private newLine: string) {
255-
this.throttledOperations = new ThrottledOperations(host);
256271
if (eventPort) {
257272
const s = net.connect({ port: eventPort }, () => {
258273
this.socket = s;
@@ -333,12 +348,26 @@ namespace ts.server {
333348
this.logger.info(`Scheduling throttled operation: ${JSON.stringify(request)}`);
334349
}
335350
}
336-
this.throttledOperations.schedule(project.getProjectName(), /*ms*/ 250, () => {
351+
352+
const operationId = project.getProjectName();
353+
const operation = () => {
337354
if (this.logger.hasLevel(LogLevel.verbose)) {
338355
this.logger.info(`Sending request: ${JSON.stringify(request)}`);
339356
}
340357
this.installer.send(request);
341-
});
358+
};
359+
const queuedRequest: QueuedOperation = { operationId, operation };
360+
361+
if (this.activeRequestCount < NodeTypingsInstaller.maxActiveRequestCount) {
362+
this.scheduleRequest(queuedRequest);
363+
}
364+
else {
365+
if (this.logger.hasLevel(LogLevel.verbose)) {
366+
this.logger.info(`Deferring request for: ${operationId}`);
367+
}
368+
this.requestQueue.push(queuedRequest);
369+
this.requestMap.set(operationId, queuedRequest);
370+
}
342371
}
343372

344373
private handleMessage(response: SetTypings | InvalidateCachedTypings | BeginInstallTypes | EndInstallTypes | InitializationFailedResponse) {
@@ -399,11 +428,39 @@ namespace ts.server {
399428
return;
400429
}
401430

431+
if (this.activeRequestCount > 0) {
432+
this.activeRequestCount--;
433+
}
434+
else {
435+
Debug.fail("Received too many responses");
436+
}
437+
438+
while (this.requestQueue.length > 0) {
439+
const queuedRequest = this.requestQueue.shift();
440+
if (this.requestMap.get(queuedRequest.operationId) === queuedRequest) {
441+
this.requestMap.delete(queuedRequest.operationId);
442+
this.scheduleRequest(queuedRequest);
443+
break;
444+
}
445+
446+
if (this.logger.hasLevel(LogLevel.verbose)) {
447+
this.logger.info(`Skipping defunct request for: ${queuedRequest.operationId}`);
448+
}
449+
}
450+
402451
this.projectService.updateTypingsForProject(response);
403452
if (response.kind === ActionSet && this.socket) {
404453
this.sendEvent(0, "setTypings", response);
405454
}
406455
}
456+
457+
private scheduleRequest(request: QueuedOperation) {
458+
if (this.logger.hasLevel(LogLevel.verbose)) {
459+
this.logger.info(`Scheduling request for: ${request.operationId}`);
460+
}
461+
this.activeRequestCount++;
462+
this.host.setTimeout(request.operation, NodeTypingsInstaller.requestDelayMillis);
463+
}
407464
}
408465

409466
class IOSession extends Session {

‎src/server/utilities.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,12 @@ namespace ts.server {
179179
constructor(private readonly host: ServerHost) {
180180
}
181181

182+
/**
183+
* Wait `number` milliseconds and then invoke `cb`. If, while waiting, schedule
184+
* is called again with the same `operationId`, cancel this operation in favor
185+
* of the new one. (Note that the amount of time the canceled operation had been
186+
* waiting does not affect the amount of time that the new operation waits.)
187+
*/
182188
public schedule(operationId: string, delay: number, cb: () => void) {
183189
const pendingTimeout = this.pendingTimeouts.get(operationId);
184190
if (pendingTimeout) {

‎src/services/signatureHelp.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -372,8 +372,7 @@ namespace ts.SignatureHelp {
372372
if (isTypeParameterList) {
373373
isVariadic = false; // type parameter lists are not variadic
374374
prefixDisplayParts.push(punctuationPart(SyntaxKind.LessThanToken));
375-
// Use `.mapper` to ensure we get the generic type arguments even if this is an instantiated version of the signature.
376-
const typeParameters = candidateSignature.mapper ? candidateSignature.mapper.mappedTypes : candidateSignature.typeParameters;
375+
const typeParameters = (candidateSignature.target || candidateSignature).typeParameters;
377376
signatureHelpParameters = typeParameters && typeParameters.length > 0 ? map(typeParameters, createSignatureHelpParameterForTypeParameter) : emptyArray;
378377
suffixDisplayParts.push(punctuationPart(SyntaxKind.GreaterThanToken));
379378
const parameterParts = mapToDisplayParts(writer =>

‎tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.typedefTagWithChildrenTags.json‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -34,38 +34,6 @@
3434
"kind": "JSDocTypeLiteral",
3535
"pos": 26,
3636
"end": 98,
37-
"jsDocTypeTag": {
38-
"kind": "JSDocTypeTag",
39-
"pos": 28,
40-
"end": 42,
41-
"atToken": {
42-
"kind": "AtToken",
43-
"pos": 28,
44-
"end": 29
45-
},
46-
"tagName": {
47-
"kind": "Identifier",
48-
"pos": 29,
49-
"end": 33,
50-
"escapedText": "type"
51-
},
52-
"typeExpression": {
53-
"kind": "JSDocTypeExpression",
54-
"pos": 34,
55-
"end": 42,
56-
"type": {
57-
"kind": "TypeReference",
58-
"pos": 35,
59-
"end": 41,
60-
"typeName": {
61-
"kind": "Identifier",
62-
"pos": 35,
63-
"end": 41,
64-
"escapedText": "Object"
65-
}
66-
}
67-
}
68-
},
6937
"jsDocPropertyTags": [
7038
{
7139
"kind": "JSDocPropertyTag",

‎tests/baselines/reference/functionConstraintSatisfaction2.errors.txt‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ tests/cases/conformance/types/typeParameters/typeArgumentLists/functionConstrain
2222
Type 'void' is not assignable to type 'string'.
2323
tests/cases/conformance/types/typeParameters/typeArgumentLists/functionConstraintSatisfaction2.ts(38,10): error TS2345: Argument of type 'U' is not assignable to parameter of type '(x: string) => string'.
2424
Type 'T' is not assignable to type '(x: string) => string'.
25-
Type '() => void' is not assignable to type '(x: string) => string'.
26-
Type 'void' is not assignable to type 'string'.
2725

2826

2927
==== tests/cases/conformance/types/typeParameters/typeArgumentLists/functionConstraintSatisfaction2.ts (13 errors) ====
@@ -102,7 +100,5 @@ tests/cases/conformance/types/typeParameters/typeArgumentLists/functionConstrain
102100
~
103101
!!! error TS2345: Argument of type 'U' is not assignable to parameter of type '(x: string) => string'.
104102
!!! error TS2345: Type 'T' is not assignable to type '(x: string) => string'.
105-
!!! error TS2345: Type '() => void' is not assignable to type '(x: string) => string'.
106-
!!! error TS2345: Type 'void' is not assignable to type 'string'.
107103
}
108104

‎tests/baselines/reference/jsdocTwoLineTypedef.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
//// [jsdocTwoLineTypedef.ts]
2+
// Regression from #18301
3+
/**
4+
* @typedef LoadCallback
5+
* @type {function}
6+
*/
7+
type LoadCallback = void;
8+
9+
10+
//// [jsdocTwoLineTypedef.js]

‎tests/baselines/reference/jsdocTwoLineTypedef.symbols‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
=== tests/cases/conformance/jsdoc/jsdocTwoLineTypedef.ts ===
2+
// Regression from #18301
3+
/**
4+
* @typedef LoadCallback
5+
* @type {function}
6+
*/
7+
type LoadCallback = void;
8+
>LoadCallback : Symbol(LoadCallback, Decl(jsdocTwoLineTypedef.ts, 0, 0))
9+

0 commit comments

Comments
 (0)