Skip to content
This repository was archived by the owner on Jan 19, 2019. It is now read-only.

How to hande declare var stataments? #185

Closed
Pajn opened this issue Mar 17, 2017 · 3 comments
Closed

How to hande declare var stataments? #185

Pajn opened this issue Mar 17, 2017 · 3 comments

Comments

@Pajn
Copy link
Contributor

Pajn commented Mar 17, 2017

Another thing I run in to when playing with prettier/prettier/issues/13 is declare var statements.

Currently prettier transforms

declare var x: any;

to

var x: any;

(the declare modifier is gone)

If we want to follow the Flow AST typescript-eslint-parser must parse it into an DeclareVariable node.
The DeclareVariable node is basically a VariableDeclarator directly (which otherwise are elements in the declarations array of VariableDeclaration).
The TS AST does instead set the DeclareKeyword as a modifier on the SyntaxKind.VariableStatement node (which usually becomes a VariableDeclaration).

A quick fix is this patch

diff --git a/lib/ast-converter.js b/lib/ast-converter.js
index 48ba730..33f025f 100644
--- a/lib/ast-converter.js
+++ b/lib/ast-converter.js
@@ -943,6 +943,24 @@ module.exports = function(ast, extra) {
                 break;

             case SyntaxKind.VariableStatement:
+                if (node.modifiers && node.modifiers.length) {
+                    var isDeclareVariable = node.modifiers.some(function(modifier) {
+                        return modifier.kind === ts.SyntaxKind.DeclareKeyword;
+                    });
+                    if (isDeclareVariable) {
+                        assign(result, {
+                            type: "DeclareVariable",
+                            id: convertChild(node.declarationList.declarations[0]),
+                            init: convertChild(node.initializer)
+                        });
+
+                        if (node.type) {
+                            result.id.typeAnnotation = convertTypeAnnotation(node.type);
+                        }
+                        break;
+                    }
+                }
+
                 assign(result, {
                     type: "VariableDeclaration",
                     declarations: node.declarationList.declarations.map(convertChild),

however it breaks quite soon as TypeScript seem to allow multiple declarations in the same statement like so:

declare var x: any, y: any;

So it seems like a new node type or a custom property on the variable declaration might be needed.

This probably isn't the highest priority issue for typescript support in prettier, I just happened to get one of those in my initial pull of test files. But I still wanted to submit an issue so it isn't forgotten :)

@soda0289
Copy link
Member

soda0289 commented Mar 17, 2017

I have PR #163 that does something similar for declared classes.

I think we should use a new node type. Something like TSAmbientVariable or TSVariableDeclaration. I don't really think we should copy the flow node types since typescript has its own unique features as you mentioned.

Pajn added a commit to Pajn/typescript-eslint-parser that referenced this issue Mar 19, 2017
Pajn added a commit to Pajn/typescript-eslint-parser that referenced this issue Apr 3, 2017
@azz
Copy link
Contributor

azz commented Aug 13, 2017

@soda0289 Do we have a solution for this issue? Turns out the way we look at the source in prettier has some ugly side-effects. prettier/prettier#2604

@JamesHenry
Copy link
Member

We can look at addressing the AST for this in the new parser project: https://github.com/JamesHenry/typescript-estree

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

No branches or pull requests

5 participants