-
-
Notifications
You must be signed in to change notification settings - Fork 75
Fix: Remove jsdoc node property from ts nodes (fixes #164) #177
Conversation
LGTM |
I looked at parsed espree and acorn nodes, they do not include the jsdoc property so I dont think we would need them on ts nodes since we dont have them on estree nodes. |
JSDoc property is not found on parsed ESTree nodes and should be removed when parsing typescript nodes.
5c9df45
to
bbd2cd6
Compare
LGTM |
I added a test |
This is currently not doing any harm right? We have started planning ESLint v4 which will contain the comment attribution work, so I think I would prefer to wait and address the comments situation holistically. Please let me know if this is actually an issue |
When we convert Class, Function, or MethodDeclaration nodes we strip out the jsDoc comment property. I do not see any harm in remove this property since it is not included on other parsers like acorn or espree. We can wait if you like but I think having this parser not fail or throw uncaught exceptions is really important. |
I could look into the typescript code base or post an issue to figure out why node.getStart() is failing? |
@weirdpattern Issue #118 was a bug in typescript where it would fail to produce tokens correctly for JSDoc comments. This issue is a little different as it deals with converting typescript JSDoc nodes into non existing ESTree nodes. |
by the way, node.getStart() fails because the node does not declare a parent... I looked into this when #118 was still open =] |
@weirdpattern I noticed that the parent property was null which means it probably is a bug in typescript, since it did work in typescript 2.0. |
Ah sorry @soda0289 I did misunderstand 😄 ! Yes let's get this in |
JSDoc property is not found on parsed ESTree nodes and should be
removed when parsing typescript nodes.