Move valid test case to invalid for no-typos due to ESLint upgrade #1437
Conversation
| @@ -763,5 +754,17 @@ ruleTester.run('no-typos', rule, { | |||
| }, { | |||
| message: 'Typo in declared prop type: objectof' | |||
| }] | |||
| }, { | |||
| // PropTypes declared on a component that is detected through JSDoc comments and is | |||
| // declared AFTER the PropTypes only works when ESLint version is >= 4.7.0 | |||
There was a problem hiding this comment.
We should support both eslint < v4.7, and >= v4.7, and this code should be made to fail as expected in both ranges.
There was a problem hiding this comment.
Hmm - not sure how we can make it fail with ESLint >= 4.7 😄
It does feel a bit strange to really force a supported case to be unsupported though? Currently we do support both version ranges, but the plugin behaves differently in both... but yes, I agree that's kind of confusing as well.
I'll see what can be done. I think the code to handle it would be like this:
- If component has JSDoc
- If static properties are declared before the component is declared
-> Then ignore propTypes typo checking
There was a problem hiding this comment.
To clarify; I think that the code warning or not warning should behave identically in 4.6 and 4.7 - I don't really care which we pick, as long as it doesn't change.
If that's not possible, then that will mean 4.7 was a breaking change, and eslint core needs to revert something so that it again works as expected.
There was a problem hiding this comment.
We definitely cannot easily detect this case with ESLint 4.6. That's why we had to choose to ignore it until the ESLint issue fixed.
Indeed, 4.7 did some kind of internal refactor of ESLint core. If you're interested, this is the change: eslint/eslint#9283
This is a reference implementation of #9122. With this change, rules don't have to worry about whether node.parent has been set at any given point, because all of the node.parent properties will be assigned by the time the rule gets run.
There was a problem hiding this comment.
in that case, we need to explicitly ignore it in eslint ^4.7.
|
This change has been reverted in |
|
Great! Sounds like this can be closed for the time being. |
With the latest release of ESLint, a case that was previously not supported is now supported. This currently fails tests on master and also for any PRs that are created.
We can't remove the work-around code yet though, because it would mean an upgrade of the ESLint peerDependency which would probably mean a major version bump?
We should remove that work-around code only when a new version is planned, right?