New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove quotes when considering the name of a proptype #1132
remove quotes when considering the name of a proptype #1132
Conversation
lib/rules/prop-types.js
Outdated
* @param {string} the identifier to strip | ||
*/ | ||
function stripQuotes(string) { | ||
if (string[0] === '\'' || string[0] === '"') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about backticks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backticked strings require computed property brackets:
{ [`complete?`]: number }
Does this not work with flow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wild.
It doesn't seem to work with flow in type declarations, and this makes sense to me. Like you say, flow would have to execute the code to compute the key name, since it might have ${}
s.
The lint rule also doesn't work for computed property names in normal propTypes:
declaration. Maybe something could be done for simple expressions, like backticks without interpolation. But I'm curious if anyone has a good use case for dynamic proptype checking anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this string always have matching quotes? or would it be better to do an unconditional replace that looks for a starting '
/"
and a matching token at the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really can't think of a way to get a key name to start with a quote, without wrapping it in quotes.
I agree it feels a bit hacky. I'll work on an update this weekend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ljharb, well, it took a bit more than a weekend, but i've updated the function as you suggest.
The added test illustrates the failing case. Proptypes with quotes were referred to by the quoted name, instead of by the bare name, for flow types.
d525a4f
to
ec94608
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
The added test illustrates the failing case. Proptypes with quotes
were referred to by the quoted name, instead of by the bare name,
for flow types.