Skip to content
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 emitter with visitors and transforms #11561

Closed
wants to merge 10 commits into from

Conversation

ivogabe
Copy link
Contributor

@ivogabe ivogabe commented Oct 12, 2016

I hope that the emitter could be exposed, like requested in #10786. I played a bit with the API, and it appeared that it could be used easily, so I made this PR. With this change, people can write tools that emit or modify JavaScript or TypeScript code. I made the following changes:

  • Added a new function emit(file: SourceFile, transformers: Transformer[] = nullTransformers, newLine = "\n")
  • Move code of emitFiles to createPrinter, so it can be used in emit and emitFiles
  • Expose most of factory.ts, in a new namespace ts.factory.
  • Expose visitEachChild
  • Expose transformers api.

Examples:

import * as ts from "typescript";

const file = ts.createSourceFile("test.ts", "function foo(x) { if (x === undefined) x = 2; return x * 3; }", ts.ScriptTarget.Latest, true);

// Visitor
const file1 = visit(file) as ts.SourceFile;

console.log(ts.emit(file1).result);

function visit(node: ts.Node): ts.Node {
    if (node.kind === ts.SyntaxKind.Identifier && (node as ts.Identifier).text === "undefined") {
        return ts.factory.createVoidZero();
    }
    return ts.visitEachChild(node, visit);
}

// Transforms
const transform: ts.Transformer = context => file => {
    context.enableSubstitution(ts.SyntaxKind.Identifier);
    context.onSubstituteNode = (emitContext, node) => {
        if (node.kind === ts.SyntaxKind.Identifier && (node as ts.Identifier).text === "undefined") {
            return ts.factory.createVoidZero();
        }
    };
    return file;
}

console.log(ts.emit(file, [transform]).result);

The result of both examples is: function foo(x) { if (x === void 0)\n x = 2; return x * 3; }

Fixes #10786

Conflicts:
	src/compiler/checker.ts
	src/compiler/emitter.ts
	src/compiler/transformers/module/module.ts
	src/compiler/transformers/module/system.ts
	src/compiler/transformers/ts.ts
	src/compiler/visitor.ts
Before this change, this would be rewritten to `namespace "typescript".factory {` by the build process, resulting in an invalid definition file.
@HerringtonDarkholme
Copy link
Contributor

@alexeagle Angular team may find this useful.

@alexeagle
Copy link
Contributor

Yes! We would love to use this. cc @evmar for angular/tsickle and @jkillian for tslint.
Also cc @chuckjaz for Angular language services

@mhegazy
Copy link
Contributor

mhegazy commented Oct 12, 2016

Thanks @ivogabe. We intend to expose the API in future releases, but we have some cleanup work that needs to be done first. We are doing this right now, so hopefully we should have more details on the time line for this change in the next few weeks.

@ivogabe
Copy link
Contributor Author

ivogabe commented Oct 12, 2016

Thanks for the reply, @mhegazy. Would it be possible to merge the PR before the cleanup, by temporarily marking all functions as internal? I guess that the problem is that the API will change during of the cleanup, but if all functions are marked as internal that wouldn't be a problem. Merging it earlier would prevent some merge conflicts and would allow me to use a custom build, that exposes everything, to experiment with the API, as I can't wait to use it.. Then I do need to accept the risk of future changes of course. Can you tell which parts will be refactored?

@mhegazy
Copy link
Contributor

mhegazy commented Oct 12, 2016

Would it be possible to merge the PR before the cleanup, by temporarily marking all functions as internal?

I would rather we clean it up first before any one uses it. i would assuming using your local copy now would be an alternative.

Can you tell which parts will be refactored?

We need to change the order of the transforms, and push some transforms out. we need to update the helper logic, and how they are handled, and we need to clean up the factory and emit interfaces.

@longlho
Copy link
Contributor

longlho commented Nov 13, 2016

@mhegazy just wanna ping you on any update for this :)

@mhegazy
Copy link
Contributor

mhegazy commented Nov 13, 2016

We have made a whole set of changes recently. including move stuff around into ES2017 and esNext transforms, moved module transformation to the correct place, etc... A few issues still remain, namely handling of helpers (@rbuckton has a PR for this), and handling of --out transformation which will require changing the API.

We also need to clean up the printer to be able to print any subtree, though this does not gate exposing the transformation api.

Hopefully we should be able to do this soon.

@dead-claudia
Copy link

dead-claudia commented Nov 19, 2016

I could see this as also very useful for implementing constant folding as a third-party module.

@ctaggart
Copy link
Contributor

Any update on this @mhegazy?

@mhegazy
Copy link
Contributor

mhegazy commented Jan 20, 2017

Handling --out is the last remaining issue. we should have this enabled soon. sorry for the delay.

@fabiandev
Copy link

fabiandev commented Jan 27, 2017

@mhegazy Is there a way to already play around with it? Or would it be an option to clone this PR and then transition to the final implementation once exposed, as I would really need this to get further in a project.

Edit:
I'm temporarily using @ivogabe's PR, which works fine for now. Thanks for that!
Looking forward to the official release :)

@ctaggart
Copy link
Contributor

Can this be put on the roadmap for 2.3? May be this is somewhat related to the #1564 Generator support for ES3/ES5.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 11, 2017

Closing in favor of #13940

@mhegazy mhegazy closed this Feb 11, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants