chore: updated dev dependencies (#4309) #4318
Conversation
jpaten
changed the title
There was a problem hiding this comment.
Hi @frano-m! When we updated sonarjs they added some new linting errors that were triggered. I wasn't sure what to do with a couple of them, so I wondered if you'd be able to take a look and let me know if what I did made sense or if another solution would be better. Thank you so much!
app/viewModelBuilders/azul/hca-dcp/common/projectMapper/projectEdits/constants.ts
Show resolved
Hide resolved
app/viewModelBuilders/azul/hca-dcp/common/projectMapper/projectEdits/constants.ts
Show resolved
Hide resolved
| "react-hooks/exhaustive-deps": "error", | ||
| "sonarjs/todo-tag": "warn", | ||
| "sonarjs/redundant-type-aliases": "warn", | ||
| "sonarjs/no-nested-conditional": "warn" |
There was a problem hiding this comment.
I changed this error to a warn since it would involve changing a lot of entries such as in announcements.tsx. @frano-m I'm not sure if we should change all of these to use chained if statements or if it's best just to disable this warn all together?
return (
<Fragment>
{isSessionTimeout ? (
<Banner onClose={clearSessionTimeout}>
For your security, you have been logged out due to 15 minutes of
inactivity.
</Banner>
) : isSystemUnavailable ? (
<Banner>
One or more of the system components are currently unavailable.
Functionality may be degraded.
</Banner>
) : isSystemIndexing ? (
<Banner>
Data indexing in progress. Downloads and exports are disabled as
search results may be incomplete.
</Banner>
) : (
generalAnnouncement
)}
</Fragment>
);
| "react-hooks/exhaustive-deps": "error" | ||
| "react-hooks/exhaustive-deps": "error", | ||
| "sonarjs/todo-tag": "warn", | ||
| "sonarjs/redundant-type-aliases": "warn", |
There was a problem hiding this comment.
@frano-m this is triggered for cases like the below in app/apis/catalog/ncpi-catalog/common/entities.ts. Not sure if it should be disabled, kept as a warn, and/or if we should remove those cases, do you have thoughts?
export type DbGapId = string;
| @@ -6,6 +6,7 @@ interface ErrorProps { | |||
| error?: Error | null; | |||
| } | |||
|
|
|||
| // eslint-disable-next-line sonarjs/no-globals-shadowing -- TODO not sure of reason for this | |||
There was a problem hiding this comment.
@frano-m I'm not super familiar with the purpose of this file - should this stay disabled, and if so would you be able to let me know a reason to specify so I can remove the TODO? Thank you so much for taking a look at these, I know there's a lot!
| @@ -201,7 +201,8 @@ export default EntityDetailPage; | |||
| * @returns catalog prefix. | |||
| */ | |||
| function getCatalogPrefix(defaultCatalog: string): string { | |||
| return defaultCatalog.replace(/\d.*$/, ""); | |||
| //eslint-disable-next-line sonarjs/slow-regex -- catalog numbers should be short and are user provided | |||
| return defaultCatalog.replace(/\d.*$/, ""); //TODO - are all catalog numbers less than a maximum length? could remove eslint ignore | |||
There was a problem hiding this comment.
@frano-m can we replace the .* here with .{0,5} or something else with a specified length to clear this error? I don't think leaving it disabled is a big deal but it might be neater
Ticket
Closes #4309.
Reviewers
@frano-m .
@hunterckx .
Changes
next-mdx-remotefrom dev to prod dependencies