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

Expose public API for transformation. #13940

Merged
merged 19 commits into from Feb 17, 2017

Conversation

Projects
None yet
@rbuckton
Member

rbuckton commented Feb 7, 2017

This PR contains the following changes:

  • TransformationContext is now public:
    • getEmitResolver and getEmitHost are still marked /*@internal*/.
    • Added assertions to ensure transformation hooks are only modified at the right times during transformation.
  • TransformationResult is now public.
  • Transformer is now public.
  • visitNode(), visitNodes(), and visitEachChild() are now public.
    • NOTE: visitEachChild() does not recursively visit TypeNode nodes, as they have so far not been necessary for emit transformations.
  • visitLexicalEnvironment(), visitParameterList(), and visitFunctionBody() helpers are now public.
  • Added the ability to attach synthetic leading and trailing comments to a node.
  • Added a customTransformers parameter to Program.emit(), to run custom transformations before and after the main transformation pipeline.
  • Added a getCustomTransformers() method to LanguageServiceHost to allow implementers to supply custom transformations to be run before and after the main transformation pipeline.
  • Added a transform() function in services that can be used to perform arbitrary transformations on source files.
    • NOTE: As the built-in transformers require the EmitResolver and EmitHost, they are not run as part of transform(), as it does not have access to these. If you require this use case consider using Program.emit() with the customTransformers parameter, or create a LanguageService from a custom LanguageServiceHost that implements the getCustomTransformers() method.

Fixes #13764
Fixes #13763

@rbuckton rbuckton requested review from mhegazy and vladima Feb 7, 2017

@rbuckton rbuckton requested a review from DanielRosenwasser Feb 7, 2017

Show outdated Hide outdated src/compiler/comments.ts
performance.mark("postEmitNodeWithSynthesizedComments");
}
debugger;

This comment has been minimized.

@mhegazy

mhegazy Feb 7, 2017

Contributor

remove.

@mhegazy

mhegazy Feb 7, 2017

Contributor

remove.

@mhegazy

This comment has been minimized.

Show comment
Hide comment
@mhegazy

mhegazy Feb 7, 2017

Contributor

//CC @alexeagle, @chuckjaz, @mprobst, @rkirov and @evmar would like to get your feedback on this change. this should allow tools like https://github.com/angular/tsickle to inject a transom either before or after the other TS transforms are applied.

//CC @ivogabe, this should supersede #11561

Contributor

mhegazy commented Feb 7, 2017

//CC @alexeagle, @chuckjaz, @mprobst, @rkirov and @evmar would like to get your feedback on this change. this should allow tools like https://github.com/angular/tsickle to inject a transom either before or after the other TS transforms are applied.

//CC @ivogabe, this should supersede #11561

@evmar

This comment has been minimized.

Show comment
Hide comment
@evmar

evmar Feb 9, 2017

Contributor

Thanks for thinking of me! I intend to look at this but I have had a lot of other work recently. If you can't wait I also don't think it'd be too harmful to land this and let us poke at it in a release. I'm not sure you have a mechanism for clearly tagging "experimental" API though.

Contributor

evmar commented Feb 9, 2017

Thanks for thinking of me! I intend to look at this but I have had a lot of other work recently. If you can't wait I also don't think it'd be too harmful to land this and let us poke at it in a release. I'm not sure you have a mechanism for clearly tagging "experimental" API though.

@rkirov

This comment has been minimized.

Show comment
Hide comment
@rkirov

rkirov Feb 10, 2017

Contributor

Unordered observations from my interaction with the API. Apologies if some stem from my misuse or misunderstanding of the API:

  • API is subtle, we can use some minimal documentation. I am confused about hints, partiallyEmittedExpression, and ran into errors of the sort Lexical environment is suspended when playing with the visit* utilities.
  • Copying the tests included, I did a mini-tsickle using purely addSynteticLeadingComment. The real implementation would not be so simple, because we always have to check and augment an existing jsdoc. That is the transformer will almost always have to modify the AST, and not just augment.
  • Completely failed to write the tsickle pass that rewrites 'export *' (needed because closures module system does not suport that) - https://github.com/angular/tsickle/blob/master/src/tsickle.ts#L402 . Despite context.enableSubstitution(ts.SyntaxKind.ExportDeclaration), I do not see onSubstituteNode being called on any such nodes.
Contributor

rkirov commented Feb 10, 2017

Unordered observations from my interaction with the API. Apologies if some stem from my misuse or misunderstanding of the API:

  • API is subtle, we can use some minimal documentation. I am confused about hints, partiallyEmittedExpression, and ran into errors of the sort Lexical environment is suspended when playing with the visit* utilities.
  • Copying the tests included, I did a mini-tsickle using purely addSynteticLeadingComment. The real implementation would not be so simple, because we always have to check and augment an existing jsdoc. That is the transformer will almost always have to modify the AST, and not just augment.
  • Completely failed to write the tsickle pass that rewrites 'export *' (needed because closures module system does not suport that) - https://github.com/angular/tsickle/blob/master/src/tsickle.ts#L402 . Despite context.enableSubstitution(ts.SyntaxKind.ExportDeclaration), I do not see onSubstituteNode being called on any such nodes.
@rbuckton

This comment has been minimized.

Show comment
Hide comment
@rbuckton

rbuckton Feb 10, 2017

Member

@rkirov While I still have some work to do on the API documentation, here are a few pointers based on your bullet points:

  • hint - TypeScript uses the same Identifier node for identifiers that are part of an expression and identifiers that are part of a declaration. When performing substitution, it is often important to know which context you are in. You should never modify the hint that is passed in to the onEmitNode or onSubstituteNode hooks, but you can inspect it to understand the context you are in:
    • EmitHint.SourceFile - Only used for a SourceFile node, this hint is used by the emitter to control how comments and source maps are emitted at the top and bottom of a source file. Generally you won't need to use it.
    • EmitHint.IdentifierName - Only used for an Identifier that is the name of a function or class declaration, this hint is used by the emitter to control how source maps are emitted for the name.
    • EmitHint.Expression - Used when emitting an expression, primarily helps to distinguish an Identifier that is in an expression position vs. one used in a declaration.
    • EmitHint.Unspecified - Used for every other node.
  • PartiallyEmittedExpression - This is a synthetic node added by transformations to apply comments and source maps for nodes that are partially removed during a transformation. For example, when TypeScript elides a TypeAssertionExpression, we replace it with a PartiallyEmittedExpression to preserve comments and source maps for the original expression.
  • visitParameterList/visitFunctionBody - When visiting a function-like declaration, we introduce a new lexical environment for the purposes of hoisting generated variable or function declarations. Due to how lexical environments and scoping rules work in JavaScript, the new lexical environment is introduced when we visit the parameter list of a function-like declaration, and is exited after the end of the function body. To ensure lexical environments are nested correctly, we can suspend any modification of a lexical environment until some point at which it is resumed, allowing us to ensure the correct state is maintained when updating the tree. It is not required to use these helpers when visiting a function, and despite their general utility I may consider removing them from the public surface area to avoid confusion. Generally you want to use visitNode, visitNodes, and visitEachChild.
  • onSubstituteNode - It depends on how you are using the transforms API. This hook provides just-in-time substitution during the final print phase, and is primarily used to substitute a relatively small subset of nodes to avoid an expensive full walk of the tree during every transformation. Since this runs during the print phase, it is quite possible some other transformation may have already replaced the ExportDeclaration with some other statement.
Member

rbuckton commented Feb 10, 2017

@rkirov While I still have some work to do on the API documentation, here are a few pointers based on your bullet points:

  • hint - TypeScript uses the same Identifier node for identifiers that are part of an expression and identifiers that are part of a declaration. When performing substitution, it is often important to know which context you are in. You should never modify the hint that is passed in to the onEmitNode or onSubstituteNode hooks, but you can inspect it to understand the context you are in:
    • EmitHint.SourceFile - Only used for a SourceFile node, this hint is used by the emitter to control how comments and source maps are emitted at the top and bottom of a source file. Generally you won't need to use it.
    • EmitHint.IdentifierName - Only used for an Identifier that is the name of a function or class declaration, this hint is used by the emitter to control how source maps are emitted for the name.
    • EmitHint.Expression - Used when emitting an expression, primarily helps to distinguish an Identifier that is in an expression position vs. one used in a declaration.
    • EmitHint.Unspecified - Used for every other node.
  • PartiallyEmittedExpression - This is a synthetic node added by transformations to apply comments and source maps for nodes that are partially removed during a transformation. For example, when TypeScript elides a TypeAssertionExpression, we replace it with a PartiallyEmittedExpression to preserve comments and source maps for the original expression.
  • visitParameterList/visitFunctionBody - When visiting a function-like declaration, we introduce a new lexical environment for the purposes of hoisting generated variable or function declarations. Due to how lexical environments and scoping rules work in JavaScript, the new lexical environment is introduced when we visit the parameter list of a function-like declaration, and is exited after the end of the function body. To ensure lexical environments are nested correctly, we can suspend any modification of a lexical environment until some point at which it is resumed, allowing us to ensure the correct state is maintained when updating the tree. It is not required to use these helpers when visiting a function, and despite their general utility I may consider removing them from the public surface area to avoid confusion. Generally you want to use visitNode, visitNodes, and visitEachChild.
  • onSubstituteNode - It depends on how you are using the transforms API. This hook provides just-in-time substitution during the final print phase, and is primarily used to substitute a relatively small subset of nodes to avoid an expensive full walk of the tree during every transformation. Since this runs during the print phase, it is quite possible some other transformation may have already replaced the ExportDeclaration with some other statement.
@rkirov

This comment has been minimized.

Show comment
Hide comment
@rkirov

rkirov Feb 14, 2017

Contributor

Thanks @rbuckton, I created mini implementations of all the transformations we want to do in tsickle and they seem to work great.

The final two questions:

  • how can one remove leading comments (tsickle will need to remove existing ones, before adding the new one)
  • how would transformers interact with source maps generation?

and one observation, TS internal API are still not strictNullChecks compatible, I had to use a lot of undefined as any when using ts.create* :)

Contributor

rkirov commented Feb 14, 2017

Thanks @rbuckton, I created mini implementations of all the transformations we want to do in tsickle and they seem to work great.

The final two questions:

  • how can one remove leading comments (tsickle will need to remove existing ones, before adding the new one)
  • how would transformers interact with source maps generation?

and one observation, TS internal API are still not strictNullChecks compatible, I had to use a lot of undefined as any when using ts.create* :)

@rbuckton

This comment has been minimized.

Show comment
Hide comment
@rbuckton

rbuckton Feb 15, 2017

Member

@rkirov:

  • how can one remove leading comments (tsickle will need to remove existing ones, before adding the new one)

There is an exported function setEmitFlags that you can use for this purpose. It only affects comments read from the source file, not synthesized comments:

ts.setEmitFlags(node, EmitFlags.NoLeadingComments);
  • how would transformers interact with source maps generation?

There is an exported function setSourceMapRange that you can use for this:

// range is a TextRange, either an existing node or some other `{ pos, end }` value.
ts.setSourceMapRange(node, range);
Member

rbuckton commented Feb 15, 2017

@rkirov:

  • how can one remove leading comments (tsickle will need to remove existing ones, before adding the new one)

There is an exported function setEmitFlags that you can use for this purpose. It only affects comments read from the source file, not synthesized comments:

ts.setEmitFlags(node, EmitFlags.NoLeadingComments);
  • how would transformers interact with source maps generation?

There is an exported function setSourceMapRange that you can use for this:

// range is a TextRange, either an existing node or some other `{ pos, end }` value.
ts.setSourceMapRange(node, range);

@rbuckton rbuckton merged commit ef25b25 into master Feb 17, 2017

4 checks passed

TypeScript Test Run typescript_node.4 Build finished.
Details
TypeScript Test Run typescript_node.6 Build finished.
Details
TypeScript Test Run typescript_node.stable Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rbuckton rbuckton deleted the publicTransformers branch Feb 17, 2017

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Apr 6, 2017

A question about API, if one have a TS source text, what API should be used to just format it?

whitecolor commented Apr 6, 2017

A question about API, if one have a TS source text, what API should be used to just format it?

@rkirov

This comment has been minimized.

Show comment
Hide comment
@rkirov

rkirov Apr 13, 2017

Contributor

@fabiandev This appears to be a regression from the last time I checked the API. We should open a separate bug for it. A minimal repro is input function f(a: () => null) {} with the identity transformer (i.e. just calling visitEachChild with (n) => n).

Contributor

rkirov commented Apr 13, 2017

@fabiandev This appears to be a regression from the last time I checked the API. We should open a separate bug for it. A minimal repro is input function f(a: () => null) {} with the identity transformer (i.e. just calling visitEachChild with (n) => n).

@fabiandev

This comment has been minimized.

Show comment
Hide comment
@fabiandev

fabiandev Apr 14, 2017

Thanks @rkirov, I've submitted a separate issue: #15192.

fabiandev commented Apr 14, 2017

Thanks @rkirov, I've submitted a separate issue: #15192.

@panesofglass

This comment has been minimized.

Show comment
Hide comment
@panesofglass

panesofglass Apr 18, 2017

Is it possible to use this API to create declaration files only? In other words, is it possible to create an AST of declarations and generate the d.ts file?

panesofglass commented Apr 18, 2017

Is it possible to use this API to create declaration files only? In other words, is it possible to create an AST of declarations and generate the d.ts file?

@mhegazy

This comment has been minimized.

Show comment
Hide comment
@mhegazy

mhegazy Apr 19, 2017

Contributor

The declaration file emit is currently not exposed separately on the API. we intend to rewrite the declaration emitter as an emit transformation in the future; once that is done we will expose that on the API as well.

Contributor

mhegazy commented Apr 19, 2017

The declaration file emit is currently not exposed separately on the API. we intend to rewrite the declaration emitter as an emit transformation in the future; once that is done we will expose that on the API as well.

@pflannery

This comment has been minimized.

Show comment
Hide comment
@pflannery

pflannery May 26, 2017

@mhegazy

The decision whether to elide the module import or not is done by the checker before all the transformations have been done.

When you say "before all transformations have been done", do you mean before custom transformations are executed?

I would like to include new source files in one of my custom transformers.
My current transformer replaces module specifiers but I can't find an API to notify the compiler to reprocess new import\referenced files during a transform

pflannery commented May 26, 2017

@mhegazy

The decision whether to elide the module import or not is done by the checker before all the transformations have been done.

When you say "before all transformations have been done", do you mean before custom transformations are executed?

I would like to include new source files in one of my custom transformers.
My current transformer replaces module specifiers but I can't find an API to notify the compiler to reprocess new import\referenced files during a transform

@mhegazy

This comment has been minimized.

Show comment
Hide comment
@mhegazy

mhegazy May 30, 2017

Contributor

I would like to include new source files in one of my custom transformers.
My current transformer replaces module specifiers but I can't find an API to notify the compiler to reprocess new import\referenced files during a transform

Transforms, all of them, happen after the checking phase has happened. you can not add new files at this point.

Contributor

mhegazy commented May 30, 2017

I would like to include new source files in one of my custom transformers.
My current transformer replaces module specifiers but I can't find an API to notify the compiler to reprocess new import\referenced files during a transform

Transforms, all of them, happen after the checking phase has happened. you can not add new files at this point.

@EisenbergEffect

This comment has been minimized.

Show comment
Hide comment
@EisenbergEffect

EisenbergEffect Feb 2, 2018

@mhegazy Did this ever get documented?

EisenbergEffect commented Feb 2, 2018

@mhegazy Did this ever get documented?

@mhegazy

This comment has been minimized.

Show comment
Hide comment
@mhegazy

mhegazy Feb 2, 2018

Contributor

We still have that on our todo list. you can find a lot of samples though in https://github.com/Microsoft/TypeScript/tree/master/src/compiler/transformers

Contributor

mhegazy commented Feb 2, 2018

We still have that on our todo list. you can find a lot of samples though in https://github.com/Microsoft/TypeScript/tree/master/src/compiler/transformers

@SerkanSipahi

This comment has been minimized.

Show comment
Hide comment
@SerkanSipahi

SerkanSipahi Mar 11, 2018

@EisenbergEffect right :)

@mhegazy i had written lot of babel-compiler plugins for javascript and postcss but now im coding for a while (because of new company) with typescript and im missing these documentation for typescript.

@mhegazy when will you guys start with the documentation?

We really need documentation like these:
babel:

postcss:

SerkanSipahi commented Mar 11, 2018

@EisenbergEffect right :)

@mhegazy i had written lot of babel-compiler plugins for javascript and postcss but now im coding for a while (because of new company) with typescript and im missing these documentation for typescript.

@mhegazy when will you guys start with the documentation?

We really need documentation like these:
babel:

postcss:

@zwwtj2014

This comment has been minimized.

Show comment
Hide comment
@zwwtj2014

zwwtj2014 Mar 20, 2018

hello @mhegazy ,

The declaration file emit is currently not exposed separately on the API. we intend to rewrite the declaration emitter as an emit transformation in the future; once that is done we will expose that on the API as well.

any plan for this?

zwwtj2014 commented Mar 20, 2018

hello @mhegazy ,

The declaration file emit is currently not exposed separately on the API. we intend to rewrite the declaration emitter as an emit transformation in the future; once that is done we will expose that on the API as well.

any plan for this?

@jpike88

This comment has been minimized.

Show comment
Hide comment
@jpike88

jpike88 Apr 20, 2018

API is still extremely difficult to figure out. Would appreciate a bit of documentation on this

jpike88 commented Apr 20, 2018

API is still extremely difficult to figure out. Would appreciate a bit of documentation on this

@SerkanSipahi

This comment has been minimized.

Show comment
Hide comment
@SerkanSipahi

SerkanSipahi Apr 20, 2018

@mhegazy any feedback will be good!

SerkanSipahi commented Apr 20, 2018

@mhegazy any feedback will be good!

@mhegazy

This comment has been minimized.

Show comment
Hide comment
@mhegazy

mhegazy Apr 20, 2018

Contributor

Sorry have not had chance to get back to adding the docs yet. #14419 tracks doing so. there are a multiple examples for what a transformation looks like in https://github.com/Microsoft/TypeScript/tree/master/src/compiler/transformers

Contributor

mhegazy commented Apr 20, 2018

Sorry have not had chance to get back to adding the docs yet. #14419 tracks doing so. there are a multiple examples for what a transformation looks like in https://github.com/Microsoft/TypeScript/tree/master/src/compiler/transformers

@SerkanSipahi

This comment has been minimized.

Show comment
Hide comment
@mhegazy

This comment has been minimized.

Show comment
Hide comment
@mhegazy

mhegazy Apr 21, 2018

Contributor

@SerkanSipahi We are not exposing these transforms on the commandline compiler (like babel does) at the time being, this proposal is tracked by #16607.
Currentlly this functionality is only exposed as part of the public API. tools that wrap the compiler, be it ts-loader, or tsickle can expose that functionality.
We do have documentation for the compiler API in https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-API, but we still missing documentation for how to use the transform pipeline. that is what #14419 is tracking.

Contributor

mhegazy commented Apr 21, 2018

@SerkanSipahi We are not exposing these transforms on the commandline compiler (like babel does) at the time being, this proposal is tracked by #16607.
Currentlly this functionality is only exposed as part of the public API. tools that wrap the compiler, be it ts-loader, or tsickle can expose that functionality.
We do have documentation for the compiler API in https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-API, but we still missing documentation for how to use the transform pipeline. that is what #14419 is tracking.

@cancerberoSgx

This comment has been minimized.

Show comment
Hide comment
@cancerberoSgx

cancerberoSgx Jun 28, 2018

Here there are three working examples of using transformation APII among other things: https://typescript-api-playground.glitch.me/.

BTW you can modify the code and run it again, have intellisense, etc. ...like a TypeScript Compiler API Playground. hope it helps.

cancerberoSgx commented Jun 28, 2018

Here there are three working examples of using transformation APII among other things: https://typescript-api-playground.glitch.me/.

BTW you can modify the code and run it again, have intellisense, etc. ...like a TypeScript Compiler API Playground. hope it helps.

@SerkanSipahi

This comment has been minimized.

Show comment
Hide comment

SerkanSipahi commented Jun 28, 2018

@cancerberoSgx thank you.

@Microsoft Microsoft locked and limited conversation to collaborators Jun 29, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.