Support read-only props in Flow for no-unused-prop-types #1390
Conversation
jseminck
force-pushed
the
no-unused-prop-types-flow-read-only-v2
branch
from
9944d30
to
2fcba06
Compare
| ...SharedPropTypes // eslint-disable-line object-shorthand | ||
| }; | ||
| `, | ||
| parser: 'babel-eslint' |
There was a problem hiding this comment.
Reformatted this test - that's why it's in the PR...
lib/rules/no-unused-prop-types.js
Outdated
| * @param {string} the identifier to strip | ||
| */ | ||
| function stripQuotes(string) { | ||
| if (string[0] === '\'' || string[0] === '"' && string[0] === string[string.length - 1]) { |
There was a problem hiding this comment.
return string.replace(/^(["']).*($1)$/, '') or similar?
lib/rules/prop-types.js
Outdated
| ? tokens[1].value | ||
| : stripQuotes(tokens[0].value) | ||
| ); | ||
| const tokens = context.getFirstTokens(node, {count: 1, filter: tokenNode => ['Identifier', 'String'].indexOf(tokenNode.type) >= 0}); |
There was a problem hiding this comment.
why would we want to strip all prefixes? only + and - exist, as far as I know.
There was a problem hiding this comment.
It seems a much safer implementation for the future. If Flow would introduce a new prefix, it should be handled automatically. (assuming that the prefix token type is similar to the types it has now).
If you think keeping a list of prefixes is better then we can go with that approach as well...
There was a problem hiding this comment.
I'd prefer to have explicit support; I'd rather it not silently risk doing the wrong thing when an unanticipated prefix exists.
I definitely don't want the plugin to crash when a new prefix is found - but i might want it to warn on the prop.
| @@ -333,10 +333,7 @@ module.exports = { | |||
| * @param {string} the identifier to strip | |||
| */ | |||
| function stripQuotes(string) { | |||
There was a problem hiding this comment.
can this repeated function be extracted out into a shared file?
There was a problem hiding this comment.
Yeah - that's what I mentioned in the other PR: prop-types and no-unused-prop-types on first sight seem to share quite a bit of logic that deals with extracting the declared prop types for a component. And currently, the logic is not in sync (e.g. props in TypedArgument for Flow is not supported at all for no-unused-prop-types.
I'll try to deal with extracting as much of the shared logic as possible this weekend. Created an issue: #1393
Fixes #1388
Added a more generic implementation that supports all prefixes, not just
-and+like it was the case in theprop-typesrule.Additionally, made both rules implement the same logic. Although, as described in the other PR, the logic for detecting prop-types should really be extracted out to a separate file. I'd like to tackle that separately as well though.