Initial Transformer Setup#279
Conversation
Added Transformer setup that allows us to incrementally port our transpile functions to transform functions. We have to prot all transpile functions that transpile Nodes that are not allowed in the 2nd AST. Once thats done we should restrucutre the whole project, because right now the transform process is just hacked before the transpilation.
| return ts | ||
| .transform(node, | ||
| [(ctx: ts.TransformationContext) => { | ||
| this.context = ctx; |
There was a problem hiding this comment.
Might be nicer to maybe put this in a static function, then create an instance of the transformer here with context as constructor argument.
There was a problem hiding this comment.
Agreed, we should replicate the pattern we use for transpiler targets (TranspilerFactory, createTranspiler...). Since we most likely want to have differentiations transformers for differentiations Lua versions in the future. But I would reorganise that some point later once we are sure how exactly the transformer architecture will be implemented.
| private context: ts.TransformationContext; | ||
|
|
||
| constructor(checker: ts.TypeChecker, options: CompilerOptions) { | ||
| this.checker = checker; |
There was a problem hiding this comment.
constructor(
private checker: ts.TypeChecker,
private context: ts.TransformationContext,
private options: CompilerOptions
) { }Is slightly nicer imo
There was a problem hiding this comment.
I think this is only nicer if all members are declared & initialized in the constructor, which is not the case here.
| switch (node.kind) { | ||
| // Declarations | ||
| case ts.SyntaxKind.ImportDeclaration: | ||
| return this.visitImportDeclaration(node as ts.ImportDeclaration); |
There was a problem hiding this comment.
Random idea: Maybe we should have default this.ignoreNode(node) (returns undefined) and this.visitDefault(node) (returns node)?
There was a problem hiding this comment.
Ideally all those method stubs should contain an actual implementation in the future. If we figure out that we don't need to transform a node (ignore it) we can just delete the case from the switch statement.
| } | ||
|
|
||
| @Test("Import named bindings exception") | ||
| public namedBindigsException(): void { |
There was a problem hiding this comment.
Maybe ignore this for now instead of removing, since we probably want to still have this test on the transformer instead of transpiler.
There was a problem hiding this comment.
This test was using a custom created AST/Node I don't think that named bindings exception can ever be thrown.
| project: | ||
| default: | ||
| target: 90 | ||
| target: 80 |
2 participants
Added Transformer setup that allows us to incrementally port our transpile functions to transform functions.
We have to port all transpile functions that transpile Nodes that are not allowed in the 2nd AST.
Once that's done we should restructure the whole project, because right now the transformation process is just hacked before the transpilation.