-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add generic type transpilation #164
Add generic type transpilation #164
Conversation
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.
Thanks for the PR! Perhaps the use of ConstructUnboundGenericType()
is due to the implementation within SourceTypeMapper.MapTo()
, but I think ConstructedFrom
is appropriate.
@@ -14,8 +15,18 @@ public SourceTypeMapper(INamedTypeSymbol sourceTypes) | |||
|
|||
public string MapTo(ITypeSymbol typeSymbol, ITranspilationOptions options) | |||
{ | |||
if (SymbolEqualityComparer.Default.Equals(typeSymbol, Assign)) | |||
if (SymbolEqualityComparer.Default.Equals(typeSymbol.GetUnboundedType(), Assign)) |
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.
if (SymbolEqualityComparer.Default.Equals(typeSymbol.GetUnboundedType(), Assign)) | |
var symbol = (typeSymbol as INamedTypeSymbol)?.ConstructedFrom ?? typeSymbol; | |
if (SymbolEqualityComparer.Default.Equals(symbol, Assign)) |
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.
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.
This is non-blocking for this PR though imo, just something we need to consider if we want to support custom type mappers in the future.
I'll have a look, I personally found it easier to wrap my head around unbounded types than having generic and concrete generic types, but I'll change it if it's really just using |
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.
Sorry for the delay in the review. LGTM!
This PR adds functionality to the type transpiler to also transpile custom generic types correctly.
For example, the following C# class
would be transpiled to this TypeScript type
This also works for nested generic types as long as the type definition is either marked as
[TranspilationSource]
or the type is known as one of the generic types with built-in support (e.g. certain collection and dictionary types,Nullable<T>
).Transpilation also works with concrete generic type arguments in inheritance chains.
I'd love to get feedback for this addition/change.