fix refreshInstance type and remove return #8478
Conversation
✅ Deploy Preview for inventree-web-pui-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8478 +/- ##
==========================================
+ Coverage 84.31% 84.33% +0.01%
==========================================
Files 1178 1178
Lines 53731 53693 -38
Branches 2022 2022
==========================================
- Hits 45305 45281 -24
+ Misses 7916 7900 -16
- Partials 510 512 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
|
@SchrodingersGat I have moved the type change made to useInstance in #8278 to a separate return, as SonarCloud flagged the change for possible conflicts. |
| refreshInstance: () => Promise<QueryObserverResult<any, any>>; | ||
| refreshInstance: () => void; | ||
| refreshInstancePromise: () => Promise<QueryObserverResult<any, any>>; |
There was a problem hiding this comment.
What is the reason for this change? You don't need to await the Promise (use its return value) if you don't want to where you use refreshInstance?
There was a problem hiding this comment.
Dosen something like:
return {
sessionData,
setSessionData,
sessionId,
- refreshSession
+ refreshSession: () => { refreshSession() },
sessionQuery,
status,would fix the error to, but without cluttering the useInstance hook with unnecessary functions. (Maybe the function needs to be wrapped in a useCallback too.)
There was a problem hiding this comment.
I do not see how this would fix the issue - please look at the errors linked in the description
There was a problem hiding this comment.
The additional wrapper promise function does seem clunky.. I'm sure there should be a better way to suppress the warning
There was a problem hiding this comment.
I do note really care how this is solved but there are 28 issues with severity middle caused by the underlying change - those should be addresses https://sonarcloud.io/project/issues?rules=typescript%3AS6544&issueStatuses=OPEN%2CCONFIRMED&id=inventree_InvenTree
Fixes a bunch of type errors ie. https://sonarcloud.io/project/issues?open=AZLptXluyTR5NnVS1-LJ&id=inventree_InvenTree