-
Notifications
You must be signed in to change notification settings - Fork 1
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
Disable compile buttons if no server connection #225
Disable compile buttons if no server connection #225
Conversation
const useIsConnected = (stanWasmServerUrl: string) => { | ||
const probeUrl = `${stanWasmServerUrl}/probe`; | ||
const [isConnected, setIsConnected] = useState<boolean>(false); | ||
const [retryCode, setRetryCode] = useState<number>(0); | ||
const retryConnection = useCallback(() => { | ||
setRetryCode((r) => r + 1); | ||
}, []); | ||
useEffect(() => { | ||
setIsConnected(false); | ||
if (!probeUrl.startsWith("http://") && !probeUrl.startsWith("https://")) { | ||
// important to do this check because otherwise fetch may succeed because | ||
// the server of this web app may respond with success | ||
return; | ||
} | ||
(async () => { | ||
try { | ||
const response = await fetch(probeUrl); | ||
if (response.status === 200) { | ||
setIsConnected(true); | ||
} | ||
} catch (err) { | ||
setIsConnected(false); | ||
} | ||
})(); | ||
}, [probeUrl, retryCode]); | ||
return { isConnected, retryConnection }; | ||
}; | ||
|
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.
Don't mean to open up a can of worms here by suggesting a rewrite of working code, but I'm wondering--now that this is moved into the CompileContext--whether this still needs to be a hook, or if it'd be more natural/clear to write it as just a couple functions (with the state/useState
held direct on the CompileContextProvider, perhaps).
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.
I kinda liked the encapsulation, but it does seem a bit odd to have a custom hook which is only used in one place, with that one place being it's own hook-like-thing.
Happy to go either way
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.
I don't have strong feelings about it, and it's working as is, so it's probably not worth any mental energy to re-envision it.
You can just ship it 😸
Closes #194