microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.94k stars 12.47k forks source link

Expose serializers from TypeScript transformer (ts.ts) #17516

Open shlomiassaf opened 7 years ago

shlomiassaf commented 7 years ago

With the introduction of the Transform API some of the logic in transformers/ts.ts needs to be exposed. (and maybe from other places...)

I'll explain by example:

@SomeDecorator()
export class MyClass {
  constructor(public myInterface: MyInterface, public myClass: MyOtherClass) {}
}

If I want to transform the code so all parameter type metadata will be on a static property of the class. (this is angular DI)

@SomeDecorator()
export class MyClass {
  constructor(public myInterface: MyInterface, public myClass: MyOtherClass) { }

  static ctorParams: [Object, MyOtherClass];
}

This operation is currently impossible.

To get the right identifier for each type I need to serialize the type with some logic so MyInterface will become Object since it's not really a value.

The native TypeScript transformer has a lot of code for handling the logic of type serialization and it's not something one will want to copy, its huge.

Even if someone decided to replicate the logic, there's not access to the EmitResolver which is the "type checker" used by the native transformer to serialize the types.

TransformationContext.getEmitResolver() is @internal

The workaround is to actually run another Program process to get access to a TypeChecker. It's not even possible to get a hold of the current running Program instance since TransformerFactory does not get access to it.

Is it possible to expose the serializatoin logic in the TypeScript transformer? I know it's complex, recursing and state dependant (scope) but it seems like not being able to serialize a type to create a literal is something basic

DanielRosenwasser commented 7 years ago

I believe @rbuckton and I have discussed the possibility of allowing users to use the checker when writing transforms. I think this would fall into that bucket.

shlomiassaf commented 7 years ago

@DanielRosenwasser I think in that context it's a lightweight checker exposed called EmitResolver right? it's @internal

Also, want to highlight that a substantial portion of the logic for serializing types (using the EmitResolver) is in the typescript transformer.

Is there a way to expose that? I mean, having the checker is a good first step but it still requires repeating code and logic that exists in TS already...

DanielRosenwasser commented 7 years ago

I think it would probably be the emit resolver like you said. At this point we'd have to discuss what the difficulties and whether that's a commitment we'd be willing to make.

tbosch commented 7 years ago

This also affects Angular / tsickle as we are now also using transformers. Our current workaround (see decorator-annotator.ts):

Get the ts.Symbol.valueDeclaration of the type

/cc @mprobst @evmar @alexeagle

tbosch commented 7 years ago

Just getting the right identifier is not enough though:

As TypeScript does its symbol analysis before the transformer phase, it will elide symbols that are only used in a type position. Our workaround above only works because we also have emitDecoratorMetadata turned on, which makes TypeScript not elide such symbols as it thinks it will emit them later in a value position.

I.e. we would also need a way to mark these identifiers / symbols as being used in a value position.

shlomiassaf commented 7 years ago

@tbosch from a brief look at the tsickle code it looks like there is no separation between non-value types (interface, type) and value types (Class).

If that is correct, it might be that an interface will end up used as a value...

I know that @ngtools/webpack does that in its loader but it doesn't throw since the loader also does ts -> JS so TS checker is out of the picture...

tbosch commented 7 years ago

@shlomiassaf we do check this here. I.e. param.type will only be filled if this is a value...

shlomiassaf commented 7 years ago

@tbosch I now see, the "real" check is here which uses a TypeChecker...

A "pure" transformer does not have an access to a TypeChecker unless the EmitResolver will get exposed in the TransformationContext (TransformationContext.getEmitResolver())

I guess that in tsickle you need a type checker anyway, but when I just want to convert constructor parameters (and param decorators, if any) to a static method (e.g. static ctorParameters() {}) I have to create my own TypeChecker which means new program for something that already exist, it's pricy.

This was exactly why I opened this issue.

Once (or if) implemented you can probably just use the serializers to get the identifier and skip a lot of logic in the decorator-annotator module

tbosch commented 7 years ago

@shlomiassaf yes, typechecking is very expensive.

Angular's ngc and tsickle are using transformers, but none of them are "pure" transformers:

For Angular we are creating an api that wraps ts.createProgram, for tsickle we just wrapped ts.Program.emit.

arciisine commented 7 years ago

I'm also facing a similar problem in my transformers. I need to know if an identifier is defined in the current file or is defined within a specific folder. Currently I have resorted to using the Language Service to provide access to the typechecker, but the performance is less than stellar. If a lightweight typechecker is to be provided, that sounds like it would fit the bill exactly.