UI: refresh plugins interval #3663
Conversation
ordabach
changed the base branch from
develop
to
3418-plugin-installation-screen
monkey/monkey_island/cc/ui/src/components/contexts/plugins/PluginsContext.tsx
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,23 @@ | |||
| import {useEffect, useRef} from 'react' | |||
|
|
|||
| export const useInterval = (callback: () => void, delay: number | null) => { | |||
There was a problem hiding this comment.
I don't think we should or need to override this.
There was a problem hiding this comment.
I don't understand why we need to wrap the useInterval and what this wrapping achieves
There was a problem hiding this comment.
a general hook to use interval with self-cleaning.
| // Remember the latest callback if it changes | ||
| useEffect(() => { | ||
| savedCallback.current = callback; | ||
| }, [callback]) |
There was a problem hiding this comment.
That's the practice
| }, [installedPlugins]); | ||
|
|
||
| const refreshAvailablePluginsAndNumberOfUpgradablePlugins = (forceRefresh = false) => { |
There was a problem hiding this comment.
In general this is not a good name. If a name is "DoSomething1andDoSomething2" it means that there are 2 functions: "DoSomething1" and "DoSomething2". So we should just call "DoSomething1" and "DoSomething2" in a row instead of creating another function. But since there's a temporal coupling, I don't feel strongly about this
There was a problem hiding this comment.
you can change it if you have a better one
This race condition happens when available plugin rows are created before the availablePlugins variable is updated.
What does this PR do?
Fixes part of #3418
PR Checklist
Testing Checklist