Excluded native JSX elements from no-unsafe-any #3699
Conversation
Interesting how TypeScript auto-added single-apostrophes...
|
|
|
Deprecation rule tests broken on testNext; otherwise passing... |
|
Hey @suchanlee just bringing this to your attention - IMO it's one of the higher priority PRs. #3080 blocks using the |
src/rules/noUnsafeAnyRule.ts
Outdated
| return isIdentifier(node) | ||
| && node.parent !== undefined | ||
| && jsxElementTypes.has(node.parent.kind) | ||
| && isLowerCase(node.text[0]); |
There was a problem hiding this comment.
instead of isLowerCase (which can allow false positives), it might be better to just get a set of all native jsx elements since they're exhaustive and well defined.
There was a problem hiding this comment.
set of all native jsx elements
I think that's no longer the case: facebook/react#2746
What kind of false positives should I be looking out for?
Off the top of my head, I'm thinking this might need to be changed to isFirstLetterLowerCase - trying to find a reference for what happens if you do something like <myElement />
There was a problem hiding this comment.
Thanks for the reminder - behaviorally, it looks like it's purely based on the first letter of the element name.
<span>lowercase span</span>
<sPan>camelCase sPan</sPan>becomes in create-react-app:
<span>lowercase span</span>
<span>camelCase sPan</span>There was a problem hiding this comment.
oh i see cool. Yeah I think it should be first letter lower case instead of lower case.
src/rules/noUnsafeAnyRule.ts
Outdated
| ]); | ||
|
|
||
| function isJsxNativeElement(node: ts.Node): boolean { | ||
| return isIdentifier(node) |
There was a problem hiding this comment.
how are fragments handled with this?
There was a problem hiding this comment.
Passes in TS > 2.1, fixing now...
src/rules/noUnsafeAnyRule.ts
Outdated
| return isIdentifier(node) | ||
| && node.parent !== undefined | ||
| && jsxElementTypes.has(node.parent.kind) | ||
| && isLowerCase(node.text[0]); |
There was a problem hiding this comment.
oh i see cool. Yeah I think it should be first letter lower case instead of lower case.
PR checklist
no-unsafe-anywith tsx throwing error #3080Overview of change:
Excludes identifiers from the rule if:
Is there anything you'd like reviewers to focus on?
I couldn't find a better way to specify these native elements, since even with
@types/reactin the type system it still comes up asany. This lower-case name checking is pretty voodoo-ish (though that is in the JSX behavior spec!).CHANGELOG.md entry:
[bugfix]
no-unsafe-anyno longer marks native JSX elements as unsafe