[Tech] Add types to all IPC functions

[CommandMC]

This PR improves our IPC system by adding return types wherever possible and refining some already existent return & parameter types
It's one of the necessary steps to finally get full confidence over our variable typings, right in line after the general IPC rework in #1783

Some non-required, but "would be nice" TODOs:

• Having some sort of linter that can automatically look at the IPC call's implementation, and make sure the paramter/return types there match up with the definition in the preload file
• Some of the GOG types could use more polish. Mostly implemented in [TECH] Move default save computation into the backend #1887, so I'd like to wait for that one to merge first

---

Use the following Checklist if you have changed something on the Backend or Frontend:

• Tested the feature and it's working on a current and clean install.
• Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
• Created / Updated Tests (If necessary)
• Created / Updated documentation (If necessary)

[flavioislima]

good stuff 👍🏽

Found critical bugs when testing

[flavioislima]

I needed to dismiss my reviews since I am having some bugs like the GamePage is not working, it is throwing an error for both GOG and Legendary:

(09:36:28) ERROR:   [Gog]:              ReferenceError: game is not defined
    at /home/flavio/Develop/Electron/HeroicGamesLauncher/build/electron/main.01dcdc58.js:11328:34
    at node:electron/js2c/browser_init:189:579
    at EventEmitter.<anonymous> (node:electron/js2c/browser_init:161:11327)
    at EventEmitter.emit (node:events:527:28)

the same happens when clicking install on the game card.

[flavioislima]

Let me know once the bugs are fixed so I can test it again 👍🏽

[CommandMC]

Looks like one of my next TODOs is "finally cutting down on the @ts-expect-error comments". This is one issue caused by them:

I renamed the game variable to appName in that function, and since the only time that was used was below a @ts-expect-error comment, I didn't get a warning that it was undefined

[flavioislima]

Looks like one of my next TODOs is "finally cutting down on the @ts-expect-error comments". This is one issue caused by them:

I renamed the game variable to appName in that function, and since the only time that was used was below a @ts-expect-error comment, I didn't get a warning that it was undefined

I see, ok I will check again 👍🏽

[flavioislima]

Tried again and I still have the same issue.
Cleaned the cache but still cannot get game info.

[BrettCleary]

The ipcMain.on/handle callback function almost always has the same function type as the preload script function. In src/common/types I described a way where we would only have to type these in the preload script instead of defining it in two different places. Here's the code/comment describing the approach.

HeroicGamesLauncher/src/common/types.ts

Lines 18 to 35 in a03347e

	// here is a way to type the callback function in ipcMain.on or ipcMain.handle
	// does not prevent callbacks with fewer parameters from being passed though
	// the microsoft team is very opposed to enabling the above constraint https://github.com/microsoft/TypeScript/issues/17868
	// for ipcMain.handle('updateGame', async (e, appName, runner) => { for instance could be converted to:
	// ipcMain.handle('updateGame', typedCallback<WrapApiFunction<typeof updateGame>>() => {
	// this has the benefit of type checking for the arguments typed in the preload api
	// but may be overly complex for a small benefit
	export function typedCallback<T>(arg: T) {
	return arg
	}
	export type WrapApiFunction<
	// eslint-disable-next-line @typescript-eslint/no-explicit-any
	TFunction extends (...args: any) => any
	> = (
	e: Electron.IpcMainInvokeEvent,
	...args: [...Parameters<TFunction>]
	) => ReturnType<TFunction>

This way we only need to change the function signature in the preload script and still get fullstack type checking.

What are your thoughts on this approach @CommandMC?

[CommandMC]

Hm, I think I'm mostly getting how this works, but I'm not sure if I like how it works 😅
IMO that can be in its own PR though, this is already getting fairly large

Thanks for the reminder though, I honestly totally missed that comment

[CommandMC]

@BrettCleary I've now implemented full-stack type checking my way. It's more compex and certainly less readable, but also has some benefits

• No more typos, you can only send/invoke and on/handle IPC channels that you've defined in the .d.ts file
• Parameter & return types are checked even in backend/api definitions (already caught some issues there)
• Parameter & return types are defined once (single source of truth)

The main drawback is that you now have to jump through one more hoop to add a new IPC function (adding it to common/typedefs/ipcBridge.d.ts, either to SyncIPCFunctions or AsyncIPCFunctions), although since this gives us more features and makes the other two hoops (adding the actual implementation in the backend, adding the callback in backend/api) easier, I'd say that's sensible

Let me know what you think

[flavioislima]

OK, looks good now! :D

[arielj]

hey, I left 2 comments, the path || defaultSteamPath one is not really that important though, I just saw it was changed in other places

[arielj]

looks good!