ref(eslint): bolster restrict-jsx-slot-children's getDisplayName#109725
ref(eslint): bolster restrict-jsx-slot-children's getDisplayName#109725JoshuaKGoldberg merged 1 commit intomasterfrom
Conversation
github-actions
bot
added
the
Scope: Frontend
| // eslint-disable-next-line consistent-return | ||
| function getDisplayName(node: TSESTree.JSXTagNameExpression): string { | ||
| // eslint-disable-next-line default-case |
There was a problem hiding this comment.
@natemoo-re are these lint rules a team opinion? It's been a while since I've worked in a TypeScript-y codebase -especially lint areas- that had them enabled.
There was a problem hiding this comment.
Yes, the lint rules represent the team opinion/house style! Most of these decisions predate me, but we are positioned to lean in on the more opinionated rules with agentic workflows.
There was a problem hiding this comment.
Ok cool! I'll try them out for a bit and update the PR to work with them.
For context, these two rules specifically are ones I've avoided because they cause false reports with TypeScript. I think they aren't necessary with types + @typescript-eslint/switch-exhaustiveness-check. But I haven't validated that in a while. This'll be a good excuse to 👍
There was a problem hiding this comment.
Ah, I did not know that! I'm fairly sure a lot of these decisions predate the Sentry codebase being written in TypeScript, fwiw. Type-aware linting was only enabled about a year ago 🙃
There was a problem hiding this comment.
I filed #109743 just to play with them. If nothing comes up in a few days I think I'll try to suggest that as a tooling fix.
| // eslint-disable-next-line consistent-return | ||
| function getDisplayName(node: TSESTree.JSXTagNameExpression): string { | ||
| // eslint-disable-next-line default-case |
There was a problem hiding this comment.
Yes, the lint rules represent the team opinion/house style! Most of these decisions predate me, but we are positioned to lean in on the more opinionated rules with agentic workflows.
JoshuaKGoldberg
force-pushed
the
eslint-restrict-jsx-slot-children-no-as
branch
from
07bb4dd to
69107cf
Compare
2 participants
Followup to #109310: modifies the logic in
restrict-jsx-slot-childrento:getDisplayNameand the ad hoc stringification in theJSXAttributevisitorI split this out into its own PR because the way I wrote
getDisplayNametriggered some reports from ESLint that I wanted to ask about.Fixes https://linear.app/getsentry/issue/ENG-6933/followup-refactor-restrict-jsx-slot-childrens-getdisplayname