feat(@ngtools/webpack): expose TS TypeChecker from ng compiler plugin

[vakrilov]

Expose the TypeScript Program created by the plugin, so that it can be used in custom platform transformers.This will allow to define custom platformTransformers similar to replace_bootstrap. They will need some way to get access to the TS program created by the plugin to make transformations.

We discussed this with @filipesilva in slack and he suggested to open a proper PR exposing the TS program and continue any discussions here.

[filipesilva]

@alexeagle @hansl @clydin I'm ok with this change, which is needed for the NativeScript team to get the TS type checker during their transforms. WDYT?

[clydin]

I'd prefer if the platformTransformers option had an additional type option. Either a platform transformers factory function or a TS transformer factory array with an expanded context object to allow for access to the type checker. An advantage is that additional information could be passed in the future if needed.

For example,

export interface PlatformTransformationContext extends ts.TransformationContext {
  getTypeChecker(): ts.TypeChecker;
}
export type PlatformTransformerFactory<T extends ts.Node = ts.SourceFile> = (context: PlatformTransformationContext) => ts.Transformer<T>;

And then the plugin options:

{
  ...
  platformTransformers: TransformerFactory<ts.SourceFile>[] | PlatformTransformerFactory[];
  ...
}

[vakrilov]

@clydin I like your suggestion, because it leaves the door open for further extensibility.

What is not clear to me is whether you have control over what context is passed to the transformers.
I can see in the code that all transformers are ultimately pushed into an array and passed to the program in emit. Isn't the TypeScript code that calls and executes the transformers?

[vakrilov]

Actually, in AOT case the AngularCompilerProgram is used, but at the end of the day it calls the emit method of the underlying ts.Program (or at least that is what I inferred form the code, might be wrong 😄). Still not sure if there is a way to pass an "extended" context to the transformers.

One other approach might be to pass in a platformTransfromersFactory (similar to the actual replaceBootstrap function) in the plugin options. It can accept a context similar to the one you proposed. Not sure if it worth to bloat the API with yet another such option though.

[clydin]

Replacing here with something similar to the following may work:

for (const platformTransformer of this._platformTransformers) {
  this._transformers.push(context => platformTransformer({ ...context, getTypeChecker }));
}

[vakrilov]

Hey @clydin I've tried this approach and as far the API goes - it works. We get the extended context.

However, this leaves us a little further form the the original goal of implementing replace_bootstrap for NativeScript projects while reusing as much code as possible 😄 . The reason fo this is that we cannot reuse the makeTransform and StandardTransforms as these actually create the TransformerFactory that gets the extended context.

In reality there are only small differences in the NativeScript bootstrap (the name of the platform and the module form where it is imported). That's why we are hopping to reuse most of the APIs so that there is less code to keep in sync.

[clydin]

The transformers are due for a general refactoring. As a result, I wouldn't be overly concerned about reuse. The current setup performs multiple full AST walks which has potential performance concerns. The need to pass getTypeChecker around is also a concern (ideally it should be part of the actual TypeScript interface). The bootstrap replacer also only performs a limited amount of AST analysis in regards to bootstrap discovery. There was a WIP PR that started some of this work for the bootstrap replacement but was unfortunately left unfinished here.

[alexeagle]

return just the typechecker here

[clydin]

this can also be unavailable during certain points in the compilation lifecycle so should also return null in those cases.

[alexeagle]

Let's keep the simple code you have, with one comment.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}