Skip to content

Reuse input nodes where possible when serializing jsdoc implements clauses - #41783

Merged
Wesley Wigham (weswigham) merged 3 commits into
microsoft:masterfrom
weswigham:implements-jsdoc-type-params
Dec 8, 2020
Merged

Reuse input nodes where possible when serializing jsdoc implements clauses#41783
Wesley Wigham (weswigham) merged 3 commits into
microsoft:masterfrom
weswigham:implements-jsdoc-type-params

Conversation

@weswigham

Copy link
Copy Markdown
Member

Fixes#41258

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about what trackExistingEntityName can do

Comment threadsrc/compiler/checker.ts Outdated
return cleanup(/*result*/ undefined);
}
}
return cleanup(factory.createExpressionWithTypeArguments(expr, map(e.typeArguments, a => serializeExistingTypeNode(context, a, includePrivateSymbol, bundled) || typeToTypeNodeHelper(getTypeFromTypeNode(a), context))));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the best way to break up this line, but it's what I ended up doing so I could read it.

Suggested change
returncleanup(factory.createExpressionWithTypeArguments(expr,map(e.typeArguments,a=>serializeExistingTypeNode(context,a,includePrivateSymbol,bundled)||typeToTypeNodeHelper(getTypeFromTypeNode(a),context))));
returncleanup(factory.createExpressionWithTypeArguments(expr,
map(e.typeArguments,a=>
serializeExistingTypeNode(context,a,includePrivateSymbol,bundled)
||typeToTypeNodeHelper(getTypeFromTypeNode(a),context))));

Comment threadsrc/compiler/checker.ts
return cleanup(factory.createExpressionWithTypeArguments(expr, map(e.typeArguments, a => serializeExistingTypeNode(context, a, includePrivateSymbol, bundled) || typeToTypeNodeHelper(getTypeFromTypeNode(a), context))));

function cleanup<T>(result: T): T {
context.enclosingDeclaration = oldEnclosing;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtb dynamic scope macro

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean... try...finally is that, but has bad perf implications last I checked.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that gives you the control flow, but not the scoping behaviour for variables. I'm talking something like special in Common Lisp, which would allow the assignment to context.enclosingDeclaration on line 6733 to be popped off the stack at the end of the scope instead of requiring the oldEnclosing=context.enclosing/context.enclosing=oldEnclosing bracketing to be manually written.

Comment threadsrc/compiler/checker.ts Outdated
const baseTypes = getBaseTypes(classType);
const implementsExpressions = mapDefined(getImplementsTypes(classType), serializeImplementedType);
const originalImplements = originalDecl && getEffectiveImplementsTypeNodes(originalDecl);
const implementsExpressions = originalImplements && sanitizeJSDocImplements(originalImplements) || mapDefined(getImplementsTypes(classType), serializeImplementedType);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be nice to break this line too

@weswigham
Wesley Wigham (weswigham) merged commit 69143ec into microsoft:masterDec 8, 2020
@weswigham
Wesley Wigham (weswigham) deleted the implements-jsdoc-type-params branch December 8, 2020 20:16
@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: TeamFor Milestone BugPRs that fix a bug with a specific milestone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generated .d.ts files from JS project are invalid

4 participants

@weswigham@sandersn@andrewbranch@typescript-bot