JamesHenry / typescript-estree

:sparkles: A parser that converts TypeScript source code into an ESTree-compatible form
https://jameshenry.blog
Other
84 stars 13 forks source link

Overview of misalignments with Babel #112

Closed armano2 closed 5 years ago

armano2 commented 5 years ago

There are still few nodes which are misaligned and we have to decide if we want to follow babel on it.

Biggest one are:

i added test cases for them in #110

Alignment test override for heritage if we want to keep our nodes we should add this to as processing in babel. ```ts ClassDeclaration(node: any) { if (node.abstract) { node.type = 'TSAbstractClassDeclaration'; delete node.abstract; } }, TSInterfaceDeclaration(node: any) { if (node.extends) { node.heritage = node.extends; delete node.extends; } else { node.heritage = []; } }, TSExpressionWithTypeArguments(node: any, parent: any) { if (parent.type === 'TSInterfaceDeclaration') { node.type = 'TSInterfaceHeritage'; } else if ( parent.type === 'ClassExpression' || parent.type === 'ClassDeclaration' ) { node.type = 'ClassImplements'; } if (node.expression) { node.id = node.expression; delete node.expression; } } ```

what we should do with this... those are breaking changes, and i'd like to hear you opinion. @JamesHenry @j-f1 @ikatyang @uniqueiniquity

armano2 commented 5 years ago

do we want to align those nodes?

JamesHenry commented 5 years ago

For TSIndexSignature do you know if there is a diagnostic which provides functionality similar to the babel check?

armano2 commented 5 years ago

Yes there is and we are reporting it already, TS1096 "An index signature must have exactly one parameter."

JamesHenry commented 5 years ago

Ok cool, sounds like the alignment there should be pretty straightforward then 👍

I don't have any strong opinions on the heritage side of things, so probably makes sense to continue the trend of matching babel unless there are obvious flaws in its AST (which obviously sometimes there is)

armano2 commented 5 years ago

ok, than i'm going to prepare PRs for that at evening, unless you want to take one of them 🐱

JamesHenry commented 5 years ago

I won't be able to until at least 7pm EST - but very happy to do it then

armano2 commented 5 years ago

ok, than i will take TSIndexSignature :)

JamesHenry commented 5 years ago

Cool, I’ll do heritage later, thanks! On Fri, 11 Jan 2019 at 11:09, Armano notifications@github.com wrote:

ok, than i will take TSIndexSignature :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JamesHenry/typescript-estree/issues/112#issuecomment-453567853, or mute the thread https://github.com/notifications/unsubscribe-auth/AA29q7EwAn0wLZyVHYG8cRDc421OwWRrks5vCLckgaJpZM4Z5r0E .

armano2 commented 5 years ago

there is one more broken part of conversion, Pattern vs Statement.

in my test project, ~600 files fails due to that :>

i was thinking about changing way how we determine it and follow on that estree spec... right now we are checking parents, and parents of parents.... and so on...

rather than that we should pass down information that we are processing pattern or statement while calling convertChild..

armano2 commented 5 years ago

but we are on good path, between versions 13.5.2 and 15.0.0 we went down with failed tests from:

image to image

btw. i tried changing mocha to jest and looks like its way way slower xd

JamesHenry commented 5 years ago

Nice, thanks for maintaining that extra test suite!

JamesHenry commented 5 years ago

We can do an updated summary of the state of the two ASTs soon 👍

armano2 commented 5 years ago

image

that's new results

JamesHenry commented 5 years ago

Awesome!