-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Do not consider namespaces when checking for DOM #2638
Conversation
f82d053
to
2453bf0
Compare
tests/lib/rules/jsx-pascal-case.js
Outdated
code: '<testComponent />' | ||
}, { | ||
code: '<test_component />' | ||
}, { |
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 don't know why these have been considered as valid in the first place. They are not in pascal case.
Is the reasoning that the problem is that they start with a lower case, and thus are wrong to begin with? I can see why we wouldn't want to let jsx-pascal-case
trigger an error in that situation.
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.
This is exactly why: 1e17259
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.
Yes, lowercase components are HTML elements, not custom ones.
About that, I saw references to the colon is a couple of places. |
I performed more tests and found this bit of code that explains it: https://github.com/babel/babel/blob/c3a5bf1ff5bd0df437bc20752464d08674d011d3/packages/babel-types/src/validators/react/isCompatTag.js#L2 I'll revert to (almost) the same RegExp and will add a comment. |
@ljharb This is ready for review. |
f545193
to
ab28224
Compare
When testing whether a component is a DOM element, we select the first portion of the name up until
:
or.
, then tests for^[a-z]|-
.There are many problems with this:
In effect it only tests the first character; not the full tagEDIT: Irrelevant-
(Related to this):
is not an allowed characterIt may be a dangerous change, but
the tests are still passing andlocally it is "more correct".UPDATE: So this triggers a "chain reaction" in
jsx-pascal-case
, which if followed solves #1334.