Should Interfaces be converted?

[Pajn]

I have started implementing support for interfaces in prettier but as the TS AST is a bit different than the ESTree it does not integrate as nice as other features have so far.

Flow does use ObjectTypeAnnotation for the body of the interface, similarly to how classes have a ClassBody node.

As the interface bodies can be quite different from my understanding (TS support a lot of (strange?) syntax in them). It might be best to keep TS prefixes on the type name even if we do this.

Just opening this for discussion, the current format is completely workable but it might at least be worth considering.

[soda0289]

I agree that we should have a TS prefix for all interface node types. There might be a benefit in the future to use the same names as flow, like sharing eslint rule, but I do not see one currently.

I do think we should use a body property inside of the TSInterfaceDeclaration as this seems to fit with the estree style. I also think then it would make sense to have a body property inside of TSInterfaceBody. Babel/Flow uses properties and Typescript uses members but ESTree uses body for class properties and I think that makes sense here.

So in summary:

TSInterfaceDeclaration {
    body: TSInterfaceBody
}
TSInterfaceBody {
    body: [ TSInterfaceProperty ]
}
TSInterfaceProperty {
    name: Identifier
    typeAnnotation: TypeAnnotation
   ....
}

I will try and spend some time and write up more documentation on the estree variant we produce as it is hard to track without reading the code.

@Pajn @JamesHenry
What do you guys think?

[Pajn]

What goes in interface bodies are very similar (or even the same?) as what goes into TypeLiters/Object types. So I think wrapping properties should be either done for both or none to avoid having two different structures for very similar code.
Personally I'm a bit against wrapping the properties because many of the nodes that goes in the interface and type literal bodies does not have a property id (for example call, construct and index signatures) which is what's extracted when wrapping the class properties.

[soda0289]

Are you saying we should not use TSInterfaceBody inside of TSInterfaceDeclaration? I was trying to mimic how estree handles Classes since an interface has similar properties to that of a abstract class, such as extending. I agree we should use the same structure for Object Types as well since they are very similar to interfaces

[Pajn]

Sorry I got a bit confused, it was TSInterfaceProperty I was thinking about.
A TSInterfaceBody would be great. What I thought was that every property inside the body was wrapped as well but that is not the case, it's just how methods are converted so ignore that.
But now I guess TSInterfaceProperty is just one of many node types that can appear inside an interface?

[soda0289]

Yes I believe we should have several node types in an TSInterfaceBody. Right now an object type or interface can have the following members: TSPropertySignature, TSMethodSignature, and TSIndexSignature. Might be more that I haven't thought of.

Maybe we should not use the name TSInterfaceProperty and just stick with TSPropertySignature. All of these would contain a name, which would be an Identifier, and optional parameters, which would contain Identifier nodes. Typescript can also have empty body functions in abstract and ambient classes and I think we should use the TSMethodSignature for those as well, unless we can come up with something better.

So to clean up what I had above:

TSInterfaceBody {
    body: [ TSPropertySignature | TSMethodSignature | TSIndexSignature ]
}

TSPropertySignature {
    name: Identifier
    typeAnnotation: TypeAnnotation
}

TSMethodSignature {
    name: Identifier
    typeAnnotation: TypeAnnotation
    parameters: [ Identifier ]
}

Thoughts?

[Pajn]

That looks great!
On top of my head is TSCallSignature and TSConstructSignature as well, and these doesn't have any identifier but that shouldn't be a problem with that scheme.

[soda0289]

Yes TSCallSignature and TSConsturctSignature as well.

I can create a PR for this, unless you have some time available.

[Pajn]

Take it if you want, but otherwise I should be able to get to it sometime this week.
Will need it for the next PR to prettier but there are other things to do as well so I'll make myself busy in either case :)

[soda0289]

I'm going to implement this today. I some free time