fix(eslint): Enable no-constant-binary-expression and fix violations #83273
Conversation
github-actions
bot
added
the
Scope: Frontend
| @@ -369,11 +369,7 @@ export function getProject( | |||
| eventData: EventData, | |||
| projects: Project[] | |||
| ): Project | undefined { | |||
| const projectSlug = (eventData?.project as string) || undefined; | |||
|
|
|||
| if (typeof projectSlug === undefined) { | |||
There was a problem hiding this comment.
typeof returns a string, it can never equal undefined... but it could equal "undefined"
There was a problem hiding this comment.
both no-constant-binary-expression and valid-typeof would catch this statement.
| const hasDifferentialFlamegraphPageFeature = organization.features.includes( | ||
| 'profiling-differential-flamegraph-page' | ||
| ); |
| @@ -41,22 +41,21 @@ export function getDocsLinkForEventType( | |||
| event: DataCategoryExact | string // TODO(isabella): get rid of strings after removing need for backward compatibility on gs | |||
| ) { | |||
| switch (event) { | |||
| case DataCategoryExact.TRANSACTION || 'transaction': | |||
There was a problem hiding this comment.
This should resolve to be DataCategoryExact.TRANSACTION || 'transaction' === DataCategoryExact.TRANSACTION therefore the right hand side of the || isn't needed.
It doesn't really matter because the enum value and the string value are the same, so this is a true statement: DataCategoryExact.TRANSACTION === 'transaction'. We don't need to write it out twice, no matter what the function types are.
| case DataCategoryExact.SPAN || | ||
| DataCategoryExact.SPAN_INDEXED || | ||
| 'span' || | ||
| 'span_indexed': |
There was a problem hiding this comment.
This wasn't working before when the function was called with DataCategoryExact.SPAN_INDEXED or span_indexed.
The case would boil down to be the same as case DataCategoryExact.SPAN only so values like "span_indexed" and "spanIndexed" wouldn't match at all and would fall through to the default case below.
| return true; | ||
| } | ||
| return false; | ||
| // TODO: do we need to return false when `newQueries[searchIndex] === undefined`? |
There was a problem hiding this comment.
I didn't want to 'fix' the existing if-statement because that would cause a change in behavior. But if we need that guard to exist, then it only needs to be if (newQueries[searchIndex] === undefined) { ...
Bundle ReportChanges will increase total bundle size by 86.42kB (0.28%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
…83273) https://eslint.org/docs/latest/rules/no-constant-binary-expression Found and fixed some bugs. Also removed some unneeded `?? ''` fallbacks. `??` is only useful when the left side could be `null` or `undefined`.
https://eslint.org/docs/latest/rules/no-constant-binary-expression
Found and fixed some bugs. Also removed some unneeded
?? ''fallbacks.??is only useful when the left side could benullorundefined.