More robust no typos fix #1375
More robust no typos fix #1375
Conversation
| const comment = sourceCode.getJSDocComment(node); | ||
| let comment; | ||
| // Sometimes the passed node may not have been parsed yet by eslint, and this function call crashes. | ||
| // Can be removed when eslint sets "parent" property for all nodes on initial AST traversal: https://github.com/eslint/eslint-scope/issues/27 |
There was a problem hiding this comment.
What about changing the places calling this method to be ":exit"?
There was a problem hiding this comment.
It does not help. The relatedComponent (in the case below it is class MyComponent) is not a child in the AST of the MemberExpression "MyComponent.PROPTYPES" - so using :exit does not work.
The reason is just that class MyComponent is declared after MyComponent.PROPTYPES and therefore eslint has not yet visited it, and therefore it does not have the parent property yet (which getJSDocComment() expects to exist).
MyComponent.PROPTYPES = {}
/** @extends React.Component */
class MyComponent extends BaseComponent {}They have a task to add parent property to all nodes on initial traversal, that would fix our issue.
lib/util/Components.js
Outdated
| @@ -218,7 +218,7 @@ function componentRule(rule, context) { | |||
| let comment; | |||
| // Sometimes the passed node may not have been parsed yet by eslint, and this function call crashes. | |||
| // Can be removed when eslint sets "parent" property for all nodes on initial AST traversal: https://github.com/eslint/eslint-scope/issues/27 | |||
| // FIXME: Remove try/catch when https://github.com/eslint/eslint-scope/issues/27 is implemented. | |||
There was a problem hiding this comment.
i think we should override the linting rule rather than avoid using "FIXME"
|
Also wrote a comment explaining the issue to the |
|
One of the eslint maintainers gave some good ideas on how to get it working properly. I'll have to re-write the rule a bit: eslint/eslint#9101 (comment) (or someone else can give it a go if they want to) |
|
With ESLint 4.7.0 we could now get rid of this patch, as However, I suppose it's not so "easy" to update our peerDependency to 4.7.0, right? That would probably require a major version bump? |
|
Yes, it would require a major version bump. |
Second attempt at fixing #1353
I think this is a better approach, but it means that currently one case is not supported. See the added test:
I suppose that is acceptable for now, at least it is better than having the rule crash.
Also explained why the try/catch is needed and linked to relevant eslint-scope issue that needs to be implemented before it can be removed.