Conversation
🦋 Changeset detectedLatest commit: cbc7ff1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5437 +/- ##
==========================================
+ Coverage 62.11% 62.16% +0.04%
==========================================
Files 1268 1269 +1
Lines 22008 22036 +28
Branches 4365 4370 +5
==========================================
+ Hits 13671 13698 +27
- Misses 8299 8300 +1
Partials 38 38 ☔ View full report in Codecov by Sentry. |
aec3ff1 to
8efd3b0
Compare
merge-reports: Run #4666
🎉 All tests passed!Github Test Reporter by CTRF 💚 🔄 This comment has been updated |
| fetchAppsWebhooks({ | ||
| variables: { | ||
| canFetchAppEvents: hasManagedAppsPermission, | ||
| canFetchAppEvents: true, |
There was a problem hiding this comment.
question: Don't you need permission check?
There was a problem hiding this comment.
It's checked outside of this hook
There was a problem hiding this comment.
Maybe would be good to add short comment about this
| } | ||
|
|
||
| return cleanup; | ||
| }, [lastInvocation, action, interval, key, setLastInvocation, skip]); |
There was a problem hiding this comment.
issue(?): if we put action in useRef (I assumed it was to avoid re-renders) why do we put it into dependencies array? This will cause to start new timeout
There was a problem hiding this comment.
It seems you're right and I'll fix it soon. It wouldn't be a big deal since new timeout will have calculated delay since last execution of the action
* fetch data at intervals * remove unnecessary Date.now() * fix type issue * add skip parameter and update tests * remove action from deps array * add comment
Scope of the change