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
[New] jsx-pascal-case
: Support unicode characters
#2557
Conversation
Can't seem to find any log or anything for why the travis build fails. But I do know there were failing tests in the project even before I started, so that could be one reason. π€ Tests related to the |
lib/rules/jsx-pascal-case.js
Outdated
const PASCAL_CASE_REGEX = /^(.*[.])*([A-Z]|[A-Z]+[a-z0-9]+(?:[A-Z0-9]+[a-z0-9]*)*)$/; | ||
const ALL_CAPS_TAG_REGEX = /^[A-Z0-9]+([A-Z0-9_]*[A-Z0-9]+)?$/; | ||
const PASCAL_CASE_REGEX = XRegExp('^(.*[.])*([\\p{Lu}]|[\\p{Lu}]+[\\p{Ll}0-9]+(?:[\\p{Lu}0-9]+[\\p{Ll}0-9]*)*)$', 'u'); | ||
const ALL_CAPS_TAG_REGEX = XRegExp('^[\\p{Lu}0-9]+([\\p{Lu}0-9_]*[\\p{Lu}0-9]+)?$', 'u'); |
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.
Test is failing because you used RegExp u
flag but it is not available in node 4. https://node.green/#ES2015-syntax-RegExp--y--and--u--flags
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.
@Svish we'll need to come up with another solution that doesn't rely on unicode mode in regexes
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.
So, support unicode characters in a way that doesn't rely on unicode? Guess I'm out and will have to just disable this rule then...
But really, why not just drop support for Node 4 and 5? They're like 4+ years old now. If someone for some reason needs to stick with Node 4 or 5, they can always stick with an eslint-plugin-react
version pre Unicode as well?
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.
Hmm, you're using XRegExp tho, which shouldn't require native unicode support.
Their age isn't relevant, people still use them; and it's not worth a breaking change for it.
However, it would be fine if the unicode recognition only worked in versions of node that support the u
flag - we could detect the support (function hasU() { try { new RegExp('o', 'u'); return true; } catch (e) { return false; } }
) and treat the tests differently.
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 I don't know how to do that though, so think someone else needs to take over here. Sorry. π
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.
Done.
Fixes #1654
Uses the
\p
Unicode character property matcher, and the tests run fine. πNote
I pulled in the
xregexp
package to definitely support\p
. The tests does seem to work without it, at least it Works On My Machineβ’, buteslint
, or something at least, doesn't seem to like it? π€