The initial motivation for the CLOCK_UPDATED action was for store.app.clock (and thus the getClockTime selector) to be a single-source-of-truth of the system time, making it easier to specify arbitrary times when writing unit tests by modifying the clock in the store. However, in practice we've just been using new Date() everywhere. If we use new Date(Date.now()) instead of new Date() whenever we need the current time (which we could do in a future refactor), it should be possible to mock out Date.now() in jest.
I think it might be useful to remove the CLOCK_UPDATED action and the getClock and getClockTime selectors:
because the CLOCK_UPDATED action also spams the Redux debug console (once every second) in a way that makes the Redux debug console harder to use (actions scroll by quickly) or less useful (actions get discarded quickly)
and because HeartbeatBackendListener shows a possible alternative of just instantiating new Date()s to get the current time from inside a React component
It looks like we are only using getClock in one place, to get the system time for the settings screen (I'm ignoring the import of getClock in OverlayScreen.tsx, because that file never actually tries to get a value from that selector; refer to https://github.com/pez-globo/pufferfish-software/issues/411#issuecomment-876695814). We are only using getClockTime to get the system time for the clock in the topbar. We're probably already using new Date() everywhere else.
In places where we need to have long-running control logic based on the system time, it might make sense to move that logic into redux-saga sagas. In particular, the heartbeat timeout logic should probably be a saga instead, since (like the other saga-related code in our frontend) it's more about the underlying connection behind the store. Restructuring the heartbeat logic this way would also make our frontend implementation more similar to the backend and firmware implementations, where heartbeat timeouts are detected by the driver for the communication protocol, rather than by application-side modules. Our saga would simply dispatch a backendConnectionLost action to be handled by react components and/or by reducers - for example, the event log reducer could simply respond to the backendConnectionLost action by deciding whether to create a new ephemeral log event about loss of the connection. This should probably remove the need for an invisible HeartbeatBackendListener component in an OverlayScreen.tsx file.
The initial motivation for the
CLOCK_UPDATEDaction was forstore.app.clock(and thus thegetClockTimeselector) to be a single-source-of-truth of the system time, making it easier to specify arbitrary times when writing unit tests by modifying the clock in the store. However, in practice we've just been usingnew Date()everywhere. If we usenew Date(Date.now())instead ofnew Date()whenever we need the current time (which we could do in a future refactor), it should be possible to mock outDate.now()in jest.I think it might be useful to remove the
CLOCK_UPDATEDaction and thegetClockandgetClockTimeselectors:CLOCK_UPDATEDaction also spams the Redux debug console (once every second) in a way that makes the Redux debug console harder to use (actions scroll by quickly) or less useful (actions get discarded quickly)HeartbeatBackendListenershows a possible alternative of just instantiatingnew Date()s to get the current time from inside a React componentIt looks like we are only using
getClockin one place, to get the system time for the settings screen (I'm ignoring the import ofgetClockinOverlayScreen.tsx, because that file never actually tries to get a value from that selector; refer to https://github.com/pez-globo/pufferfish-software/issues/411#issuecomment-876695814). We are only usinggetClockTimeto get the system time for the clock in the topbar. We're probably already usingnew Date()everywhere else.In places where we need to have long-running control logic based on the system time, it might make sense to move that logic into redux-saga sagas. In particular, the heartbeat timeout logic should probably be a saga instead, since (like the other saga-related code in our frontend) it's more about the underlying connection behind the store. Restructuring the heartbeat logic this way would also make our frontend implementation more similar to the backend and firmware implementations, where heartbeat timeouts are detected by the driver for the communication protocol, rather than by application-side modules. Our saga would simply dispatch a
backendConnectionLostaction to be handled by react components and/or by reducers - for example, the event log reducer could simply respond to thebackendConnectionLostaction by deciding whether to create a new ephemeral log event about loss of the connection. This should probably remove the need for an invisibleHeartbeatBackendListenercomponent in anOverlayScreen.tsxfile.