A bit of redux housekeeping

We've had this weird function signature in the core of the redux state 
management. In this commit we remove that extra function, while 
preserving reactivity as is.

Also cleans up a few typos, and reduces a bit of code duplication.

Author: mtsgrd

apps/desktop/src/lib/state/clientState.svelte.ts

@@ -24,7 +24,7 @@ import type { IrcClient } from '$lib/irc/ircClient.svelte';
 import type { ReduxError } from '$lib/state/reduxError';
 
 /**
- * GitHub API object that enables the declaration and usage of endpoints
+ * Backend API object that enables the declaration and usage of endpoints
  * colocated with the feature they support.
  */
 export type BackendApi = ReturnType<typeof createBackendApi>;
@@ -78,9 +78,8 @@ export class ClientState {
 		posthog: PostHogWrapper
 	) {
 		const butlerMod = butlerModule({
-			// Reactive loop without nested function.
-			// TODO: Can it be done without nesting?
-			getState: () => () => this.rootState as any as RootState<any, any, any>,
+			// Returns a function that returns the current state (required by butlerModule API)
+			getState: () => this.rootState as unknown as RootState<any, any, any>,
 			getDispatch: () => this.dispatch,
 			posthog
 		});
@@ -177,7 +176,6 @@ function createStore(params: {
 		}
 	});
 
-	// persistStore(store);
 	return { store, reducer };
 }
 
@@ -207,8 +205,8 @@ function createBackendApi(butlerMod: ReturnType<typeof butlerModule>) {
 }
 
 // Default cache expiration for unused items is 60 seconds. This is too little
-// for forge data.
-const KEEP_UNUSED_SECONDS = 24 * 60 * 60;
+// for forge data, so we keep forge data cached for 24 hours.
+const FORGE_CACHE_TTL_SECONDS = 24 * 60 * 60; // 24 hours
 
 // Fake base query that allows us to use the same error type when the query
 // definitions only use `queryFn` instead of `query`.
@@ -217,22 +215,25 @@ const fakeBaseQuery: BaseQueryFn = () => {
 	return { data: undefined } as QueryReturnValue<never, ReduxError, any>;
 };
 
+// Common API configuration for forge APIs
+const FORGE_API_CONFIG = {
+	tagTypes: Object.values(ReduxTag),
+	invalidationBehavior: 'immediately' as const,
+	baseQuery: fakeBaseQuery,
+	refetchOnFocus: true,
+	refetchOnReconnect: true,
+	keepUnusedDataFor: FORGE_CACHE_TTL_SECONDS,
+	endpoints: (_: any) => ({})
+};
+
 export function createGitHubApi(butlerMod: ReturnType<typeof butlerModule>) {
 	return buildCreateApi(
 		coreModule(),
 		butlerMod
 	)({
 		reducerPath: 'github',
-		tagTypes: Object.values(ReduxTag),
-		invalidationBehavior: 'immediately',
-		// TODO: This should only be set for backend api.
-		baseQuery: fakeBaseQuery,
-		refetchOnFocus: true,
-		refetchOnReconnect: true,
-		keepUnusedDataFor: KEEP_UNUSED_SECONDS,
-		endpoints: (_) => {
-			return {};
-		}
+		// Using fake base query for forge APIs (GitHub/GitLab) since they use queryFn
+		...FORGE_API_CONFIG
 	});
 }
 
@@ -242,15 +243,7 @@ function createGitLabApi(butlerMod: ReturnType<typeof butlerModule>) {
 		butlerMod
 	)({
 		reducerPath: 'gitlab',
-		tagTypes: Object.values(ReduxTag),
-		invalidationBehavior: 'immediately',
-		// TODO: This should only be set for backend api.
-		baseQuery: fakeBaseQuery,
-		refetchOnFocus: true,
-		refetchOnReconnect: true,
-		keepUnusedDataFor: KEEP_UNUSED_SECONDS,
-		endpoints: (_) => {
-			return {};
-		}
+		// Using fake base query for forge APIs (GitHub/GitLab) since they use queryFn
+		...FORGE_API_CONFIG
 	});
 }

apps/desktop/src/lib/state/context.ts

@@ -1,6 +1,6 @@
 import type { PostHogWrapper } from '$lib/analytics/posthog';
-import type { EntityState, ThunkDispatch, UnknownAction } from '@reduxjs/toolkit';
-import type { CombinedState } from '@reduxjs/toolkit/query';
+import type { ThunkDispatch, UnknownAction } from '@reduxjs/toolkit';
+import type { RootState } from '@reduxjs/toolkit/query';
 
 /**
  *	The api is necessary to create the store, so we need to provide
@@ -11,7 +11,7 @@ import type { CombinedState } from '@reduxjs/toolkit/query';
  */
 export type HookContext = {
 	/** Without the nested function we get looping reactivity.  */
-	getState: () => () => { [k: string]: CombinedState<any, any, any> | EntityState<any, any> };
+	getState: () => RootState<any, any, any>;
 	getDispatch: () => ThunkDispatch<any, any, UnknownAction>;
 	posthog?: PostHogWrapper;
 };

apps/desktop/src/lib/state/customHooks.svelte.ts

@@ -14,7 +14,6 @@ import {
 	type QueryActionCreatorResult,
 	type QueryArgFrom,
 	type ResultTypeFrom,
-	type RootState,
 	type StartQueryActionCreatorOptions
 } from '@reduxjs/toolkit/query';
 import { createSubscriber } from 'svelte/reactivity';
@@ -49,7 +48,6 @@ export function buildQueryHooks<Definitions extends ExtensionDefinitions>({
 	ctx: HookContext;
 }) {
 	const endpoint = api.endpoints[endpointName]!;
-	const state = getState() as any as () => RootState<any, any, any>;
 
 	const { initiate, select } = endpoint as ApiEndpointQuery<CustomQuery<any>, Definitions>;
 
@@ -128,7 +126,7 @@ export function buildQueryHooks<Definitions extends ExtensionDefinitions>({
 		}
 
 		const selector = $derived(select(queryArg));
-		const result = $derived(selector(state()));
+		const result = $derived(selector(getState()));
 
 		const output = $derived.by(() => {
 			let data = result.data;
@@ -181,7 +179,7 @@ export function buildQueryHooks<Definitions extends ExtensionDefinitions>({
 
 		const results = queryArgs.map((queryArg) => {
 			const selector = $derived(select(queryArg));
-			const result = $derived(selector(state()));
+			const result = $derived(selector(getState()));
 			const output = $derived.by(() => {
 				let data = result.data;
 				if (options?.transform && data) {
@@ -202,7 +200,7 @@ export function buildQueryHooks<Definitions extends ExtensionDefinitions>({
 
 	function useQueryState<T extends TranformerFn>(queryArg: unknown, options?: { transform?: T }) {
 		const selector = $derived(select(queryArg));
-		const result = $derived(selector(state()));
+		const result = $derived(selector(getState()));
 		const output = $derived.by(() => {
 			let data = result.data;
 			if (options?.transform && data) {
@@ -219,7 +217,7 @@ export function buildQueryHooks<Definitions extends ExtensionDefinitions>({
 
 	function useQueryTimeStamp(queryArg: unknown) {
 		const selector = $derived(select(queryArg));
-		const result = $derived(selector(state()));
+		const result = $derived(selector(getState()));
 		return reactive(() => result.startedTimeStamp);
 	}
 
@@ -344,7 +342,6 @@ export function buildMutationHook<
 	ctx: HookContext;
 }): MutationHook<D> {
 	const endpoint = api.endpoints[endpointName]!;
-	const state = getState() as any as () => RootState<any, any, any>;
 
 	const { initiate, select } = endpoint as unknown as ApiEndpointMutation<D, Definitions>;
 
@@ -454,7 +451,7 @@ export function buildMutationHook<
 		}
 
 		const selector = $derived(select({ requestId: promise?.requestId, fixedCacheKey }));
-		const result = $derived(selector(state()));
+		const result = $derived(selector(getState()));
 
 		const subscribe = createSubscriber(() => {
 			return () => {

apps/desktop/src/lib/testing/mockGitHubApi.svelte.ts

@@ -31,7 +31,7 @@ export function setupMockGitHubApi() {
 	const gitHubClient = new GitHubClient({ client: octokit });
 	gitHubClient.setRepo({ owner: 'test-owner', repo: 'test-repo' });
 	const gitHubApi = createGitHubApi(
-		butlerModule({ getDispatch: () => dispatch!, getState: () => () => state })
+		butlerModule({ getDispatch: () => dispatch!, getState: () => state })
 	);
 
 	const store = configureStore({