chore(website): fixed most no-unnecessary-condition complaints #7836
Conversation
|
Thanks for the PR, @JoshuaKGoldberg! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
bradzacher
added
the
repo maintenance
bradzacher
added
the
1 approval
| @@ -267,7 +267,7 @@ export const LoadedEditor: React.FC<LoadedEditorProps> = ({ | |||
| return debounce(() => editor.layout(), 1); | |||
| }, [editor]); | |||
|
|
|||
| const container = editor.getContainerDomNode?.() ?? editor.getDomNode(); | |||
| const container = editor.getContainerDomNode(); | |||
There was a problem hiding this comment.
this is not correct, as this part of code is neccesary for some older versions of playground (older version of ts)
There was a problem hiding this comment.
🤔 I'm confused, where are those types defined? The editor.api.d.ts I see has getContainerDomNode(): HTMLElement; - and I thought we're settled on just one editor version?
| @@ -50,7 +50,7 @@ export function getRuleJsonSchemaWithErrorLevel( | |||
| }; | |||
| } | |||
| // example: naming-convention rule | |||
| if (typeof ruleSchema.items === 'object' && ruleSchema.items) { | |||
| if (typeof ruleSchema.items === 'object') { | |||
There was a problem hiding this comment.
typeof null === 'object' // true
There was a problem hiding this comment.
I think in the context of the types it's probably defined as items?: T[] - hence the typeof is enough to refine the types correctly?
This was a strictness change we added in v6.
We could probably even change this check to be "cleaner" but it'll do fine like this I think
There was a problem hiding this comment.
Yeah I don't see any null values for items in any ESLint or typescript-eslint rule. Is there a reason to support it?
| @@ -67,7 +67,7 @@ export function TypeInfo({ | |||
| onHoverNode, | |||
| }: TypeInfoProps): React.JSX.Element { | |||
| const computed = useMemo(() => { | |||
| if (!typeChecker || !value) { | |||
| if (!typeChecker) { | |||
There was a problem hiding this comment.
@armano2 according to the types it is - the property value is defined as readonly value: Node and it's never reassigned in the component body
| @@ -48,15 +48,13 @@ export function TypesDetails({ | |||
| value={value} | |||
| /> | |||
| </div> | |||
| {selectedNode && ( | |||
There was a problem hiding this comment.
@armano2 based on the types it can't be.
the initial value passed to useState is typed as readonly value: Node.
Then the only call to setSelectedNode is guarded by if (item.node) { setSelectedNode(item.node) }
unless, of course, SimplifiedTreeView can make it undefined? But based on its types it can't do that I believe (or else there would be a type error on its onSelect prop with this change)
JoshuaKGoldberg
changed the title
JoshuaKGoldberg
deleted the
lint-enable-no-unnecessary-condition-website
branch
PR Checklist
Overview
Makes progress on fixing existing
@typescript-eslint/no-unnecessary-conditionviolations underpackages/website. Part of a series of PRs that split this up, as there are many of them.