-
Notifications
You must be signed in to change notification settings - Fork 12.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mark as referenced aliases in Union that will get emitted as part of decorator metadata #13540
Conversation
src/compiler/checker.ts
Outdated
if (node) { | ||
switch (node.kind) { | ||
case SyntaxKind.IntersectionType: | ||
case SyntaxKind.UnionType: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we allow unions and intersections any ways. we do not serialize them correctly anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we allow this if the Individual type leads to same identifier. This is to make sure string | null will emit type as string.. E | null results in E and "foo" | "bar" results in string as type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#13034 added support for E | null to emit it as type in metadata as E
src/compiler/checker.ts
Outdated
|
||
type voidUndefinedNullOrNeverTypeNode = Token<SyntaxKind.VoidKeyword | SyntaxKind.UndefinedKeyword | SyntaxKind.NullKeyword | SyntaxKind.NeverKeyword>; | ||
|
||
function getEntityNameForDecoratoryMetadata(node: TypeNode): EntityName | voidUndefinedNullOrNeverTypeNode { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'getEntityNameForDecoratorMetadata'
src/compiler/checker.ts
Outdated
@@ -16743,6 +16746,65 @@ namespace ts { | |||
} | |||
} | |||
|
|||
function markDecoratorMedataDataTypeNodeAsReferenced(node: TypeNode): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for consistency markDecoratorMedataDataTypeNodeAsReferenced
should be markDecoratorMedataDataTypeNodeAsReference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add comment on why we need this separate from using markTypeNodeAsReferenced
src/compiler/checker.ts
Outdated
for (const typeNode of (<UnionOrIntersectionTypeNode>node).types) { | ||
const individualEntityName = getEntityNameForDecoratoryMetadata(typeNode); | ||
if (!individualEntityName) { | ||
// Individual is something like string number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this mean that if each constituent of intersection or union type is a string or number, then return undefined?
Can you note where it will get seriazlied?
src/compiler/checker.ts
Outdated
case SyntaxKind.UndefinedKeyword: | ||
case SyntaxKind.NullKeyword: | ||
case SyntaxKind.NeverKeyword: | ||
return <voidUndefinedNullOrNeverTypeNode>node; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we return the token here? This function is only used by markDecoratorMedataDataTypeNodeAsReferenced
, which in turn only cares if its an EntityName
.
src/compiler/transformers/ts.ts
Outdated
@@ -1739,6 +1739,8 @@ namespace ts { | |||
} | |||
|
|||
function serializeUnionOrIntersectionType(node: UnionOrIntersectionTypeNode): SerializedTypeNode { | |||
// Note when updating logic here also update getEntityNameForDecoratoryMetadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getEntityNameForDecoratorMetadata
OneType|null is again treated as object instead of simplifying it
Merging this as per @mhegazy 's offline approval. |
Fixes #13449