feat: leverage slippi backend's getLatestDolphin query #271
Conversation
| const appVersion = __VERSION__; | ||
|
|
||
| const client = new ApolloClient({ | ||
| link: httpLink, |
There was a problem hiding this comment.
Why do we need to provide a link here instead of just providing the uri directly? Does it not work if we don't provide it an instance to cross-fetch?
There was a problem hiding this comment.
Yep, we get this explicit error from ApolloClient
Invariant Violation:
"fetch" has not been found globally and no fetcher has been configured. To fix this, install a fetch package (like https://www.npmjs.com/package/cross-fetch), instantiate the fetcher, and pass it into your HttpLink constructor. For example:
import fetch from 'cross-fetch';
import { ApolloClient, HttpLink } from '@apollo/client';
const client = new ApolloClient({
link: new HttpLink({ uri: '/graphql', fetch })
});
| await assertDolphinInstallation(DolphinLaunchType.NETPLAY, logDownloadInfo); | ||
| await assertDolphinInstallation(DolphinLaunchType.PLAYBACK, logDownloadInfo); |
There was a problem hiding this comment.
Ideally we could use Promise.all so we could potentially avoid a waterfall. However, since we're sending the log to logDownloadInfo we might not be able to do that, since it would intersperse the log messages. Something we could keep in mind for the future though.
src/dolphin/downloadDolphin.ts
Outdated
| @@ -27,106 +39,63 @@ export async function assertDolphinInstallation( | |||
| await findDolphinExecutable(type); | |||
| log(`Found existing ${type} Dolphin executable.`); | |||
| log(`Checking if we need to update ${type} Dolphin`); | |||
| const data = await getLatestReleaseData(type); | |||
| const latestVersion = data.tag_name; | |||
| const data = await fetchLatestDolphin(type); | |||
There was a problem hiding this comment.
| const data = await fetchLatestDolphin(type); | |
| const dolphinDownloadInfo = await fetchLatestDolphin(type); |
Can we use a better variable name than data here?
src/dolphin/downloadDolphin.ts
Outdated
| } | ||
|
|
||
| async function downloadLatestDolphin( | ||
| type: DolphinLaunchType, | ||
| releaseInfo: DolphinVersionResponse, |
There was a problem hiding this comment.
| releaseInfo: DolphinVersionResponse, | |
| downloadUrl: string, |
Can we pass the URL in directly here?
| await download(win, downloadUrl, { | ||
| filename: filename, |
There was a problem hiding this comment.
This is just a comment, but I think it would be good if we could eventually not depend on the existence of a browser window to download a file since electron-dl seems to require a Browser window. I feel like there are a few cases where the app is running without a window (e.g. MacOS) or in the task bar/system tray (perhaps in the future).
Swaps our current implementation of querying github in favor of the getLatestDolphin query on our backend.
The refactor of
downloadDolphin.tsdoes reorder a bit of the code so the diff is a bit greater there.