Extract auto refresh from stores #2658
Conversation
src/Frontend/src/App.vue
Outdated
| onMounted(async () => { | ||
| await useServiceControl(); | ||
| useServiceControlAutoRefresh(); |
There was a problem hiding this comment.
Made this initialisation of background refresh tasks explicit so that they get cleaned up as part of unmount.
| ); | ||
| onUnmounted(() => { | ||
| dataRetriever.updateTimeout(null); |
There was a problem hiding this comment.
This was just to stop the autorefresh; it is no longer needed. We do this differently now.
| onUnhandledRequest: (request, { error }) => { | ||
| console.log("Unhandled %s %s", request.method, request.url); | ||
| error(); | ||
| onUnhandledRequest: (request) => { |
There was a problem hiding this comment.
Decided to reduce this to just a warning, given that we don't really need to blow up.
|
|
||
| return ( | ||
| "default-src 'none';" + | ||
| "default-src 'self';" + |
There was a problem hiding this comment.
This is to enable the Vue Devtools to render correctly.
This CSP is only for dev.
| globals: true, | ||
| clearMocks: true, | ||
| css: true, | ||
| testTimeout: 10000, |
There was a problem hiding this comment.
Decided to slightly increase the timeout after reviewing the times on the CI.
They seem to take longer randomly.
This file is auto generated by msw
| const historyPeriodStore = useMonitoringHistoryPeriodStore(); | ||
|
|
||
| const getMemoisedEndpointDetails = memoiseOne(MonitoringEndpoints.useGetEndpointDetails); | ||
| const getMemoisedEndpointDetails = useMemoize(MonitoringEndpoints.useGetEndpointDetails); |
There was a problem hiding this comment.
i read https://vueuse.org/core/useMemoize/ to see if it's different to memoiseOne, since memoiseOne is specifically designed to only ever keep one invocation cached, and it looks like it's the same? A bit unclear... but shouldn't really matter either way
Added more diagnostics so we can see what is going on
This test is just validating what SC does. No need for it.
| const isLoading = ref(false); | ||
| const dataRetriever = useAutoRefresh( | ||
| throttle(async () => { |
There was a problem hiding this comment.
was this throttle carried through? I think it was intended to stop another refresh from happening before the current running one finished
There was a problem hiding this comment.
I didn't carried it over on purpose, I honestly don't remmeber why I added it
There was a problem hiding this comment.
With the new implementation, there shouldn't be 2 refreshes happening at the same time anyway
| mockServer.listen({ | ||
| onUnhandledRequest: (request, { error }) => { | ||
| console.log("Unhandled %s %s", request.method, request.url); | ||
| error(); |
There was a problem hiding this comment.
it doesn't need to error?
There was a problem hiding this comment.
I don't see why.
At the moment, with the error, it just means that we forgot to mock a response, but that should fail a test. If that is true, then why do we need to error at all?
If the tests are well written, the test will fail. Does this make sense @PhilBastian ?
2 participants
This PR re-enabled the latest Windows CI runner.
In this PR, we remove the auto-refresh from the stores and instead create a composable that uses
onmountandunmountedto start and stop the auto-refresh, which is then bound to a component.We also standardised on
vueuselib for things like throttling and debouncing, ... This library is more in line with Vue development practices instead of lodash and other libs.I also took this opportunity to add Vue DevTools, which helps with debugging Vue apps in the browser. This can be seen as an extension of the browser's developer tools. This is only available when running in Vite locally.