-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: handle low cli-app-scripts version #1349
Changes from all commits
8f0255c
43d070d
2542c1f
45759ac
a3ab61d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -181,6 +181,16 @@ export const Dhis2ConnectionStatusProvider = ({ | |
}, [pingAndHandleStatus, serverVersion]) | ||
|
||
useEffect(() => { | ||
if (!offlineInterface.subscribeToDhis2ConnectionStatus) { | ||
// Missing this functionality from the offline interface -- | ||
// use a ping on startup to get the status | ||
smartIntervalRef.current?.invokeCallbackImmediately() | ||
console.warn( | ||
'Please upgrade to @dhis2/cli-app-scripts@>10.3.8 for full connection status features' | ||
) | ||
return | ||
} | ||
|
||
Comment on lines
+184
to
+193
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This bit protects against the compatibility error |
||
const unsubscribe = offlineInterface.subscribeToDhis2ConnectionStatus({ | ||
onUpdate, | ||
}) | ||
|
@@ -190,23 +200,23 @@ export const Dhis2ConnectionStatusProvider = ({ | |
}, [offlineInterface, onUpdate]) | ||
|
||
// Memoize this value to prevent unnecessary rerenders of context provider | ||
const contextValue = useMemo( | ||
() => ({ | ||
// in the unlikely circumstance that offlineInterface.latestIsConnected | ||
// is `null` when this initializes, fail safe by defaulting to | ||
// `isConnected: false` | ||
isConnected: Boolean(isConnected), | ||
isDisconnected: !isConnected, | ||
lastConnected: isConnected | ||
const contextValue = useMemo(() => { | ||
// in the unlikely circumstance that offlineInterface.latestIsConnected | ||
// is `null` or `undefined` when this initializes, fail safe by defaulting to | ||
// `isConnected: true`. A ping or SW update should update the status shortly. | ||
const validatedIsConnected = isConnected ?? true | ||
return { | ||
isConnected: validatedIsConnected, | ||
isDisconnected: !validatedIsConnected, | ||
lastConnected: validatedIsConnected | ||
Comment on lines
-193
to
+211
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a small fix: the correct fail-safe should be to default to |
||
? null | ||
: // Only evaluate if disconnected, since local storage | ||
// is synchronous and disk-based. | ||
// If lastConnected is not set in localStorage though, set it. | ||
// (relevant on startup) | ||
getLastConnected(appName) || updateLastConnected(appName), | ||
}), | ||
[isConnected, appName] | ||
) | ||
} | ||
}, [isConnected, appName]) | ||
|
||
return ( | ||
<Dhis2ConnectionStatusContext.Provider value={contextValue}> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the fix to the fail-safe below