Improve isFetching reliability (HoudiniGraphql#686)

* ⚡ FIX: setConfig already done in BaseStore

* 🚸 IMPROVE: isFetching to happen only when a network call really happen.

* ✏️ DOC: changeset

* ✏️ DOC: changeset

* ✅ UPDATE: with tests

* track false fetching state when loading pages

Co-authored-by: Alec Aivazis <[email protected]>

.changeset/sharp-ravens-hear.md

@@ -0,0 +1,7 @@
+---
+'houdini': patch
+'houdini-react': patch
+'houdini-svelte': patch
+---
+
+isFetching will switch only when a network call is happening (and starts at true for queries)

e2e/sveltekit/src/lib/utils/routes.ts

@@ -4,6 +4,8 @@ export const routes = {
 
   // features
   Query_param: '/query-param',
+  isFetching_with_load: '/isFetching/with_load',
+  isFetching_without_load: '/isFetching/without_load',
 
   Stores_SSR: '/stores/ssr',
   Stores_Network: '/stores/network',

e2e/sveltekit/src/routes/isFetching/spec.ts

@@ -0,0 +1,60 @@
+import { expect, test } from '@playwright/test';
+import { routes } from '../../lib/utils/routes.js';
+import { clientSideNavigation, goto, locator_click } from '../../lib/utils/testsHelper.js';
+
+test.describe('isFetching', () => {
+  test('with_load SSR', async ({ page }) => {
+    const [msg] = await Promise.all([
+      page.waitForEvent('console'),
+      goto(page, routes.isFetching_with_load)
+    ]);
+
+    expect(msg.text()).toBe('with_load - isFetching: false');
+  });
+
+  test('with_load CSR', async ({ page }) => {
+    await goto(page, routes.Home);
+
+    // Switch page and check directly the first console log
+    const [msg] = await Promise.all([
+      page.waitForEvent('console'),
+      clientSideNavigation(page, routes.isFetching_with_load)
+    ]);
+    expect(msg.text()).toBe('with_load - isFetching: true');
+
+    // wait for the isFetching false
+    const msg2 = await page.waitForEvent('console');
+    expect(msg2.text()).toBe('with_load - isFetching: false');
+  });
+
+  test('without_load CSR', async ({ page }) => {
+    await goto(page, routes.Home);
+
+    // Switch page and check the first console log
+    // It's expected to stay true until the first fetch!
+    const [msg] = await Promise.all([
+      page.waitForEvent('console'),
+      clientSideNavigation(page, routes.isFetching_without_load)
+    ]);
+    expect(msg.text()).toBe('without_load - isFetching: true');
+
+    const [msg2] = await Promise.all([
+      page.waitForEvent('console'),
+      // manual fetch
+      locator_click(page, 'button')
+    ]);
+    expect(msg2.text()).toBe('without_load - isFetching: true');
+
+    // wait for the isFetching false
+    const msg3 = await page.waitForEvent('console');
+    expect(msg3.text()).toBe('without_load - isFetching: false');
+
+    // second click should not refetch... so isFetching should be false
+    const [msg4] = await Promise.all([
+      page.waitForEvent('console'),
+      // manual fetch
+      locator_click(page, 'button')
+    ]);
+    expect(msg4.text()).toBe('without_load - isFetching: false');
+  });
+});

e2e/sveltekit/src/routes/isFetching/with_load/+page.svelte

@@ -0,0 +1,16 @@
+<script lang="ts">
+  import { graphql } from '$houdini';
+
+  const store = graphql`
+    query isFetching_w {
+      user(id: 1, snapshot: "isFetching_w", delay: 200) {
+        id
+        name
+      }
+    }
+  `;
+
+  $: console.log(`with_load - isFetching: ${$store.isFetching}`);
+</script>
+
+<pre>{JSON.stringify($store, null, 2)}</pre>

e2e/sveltekit/src/routes/isFetching/without_load/+page.svelte

@@ -0,0 +1,22 @@
+<script lang="ts">
+  import { graphql } from '$houdini';
+
+  const store = graphql`
+    query isFetching_wo @houdini(load: false) {
+      user(id: 1, snapshot: "isFetching_wo", delay: 200) {
+        id
+        name
+      }
+    }
+  `;
+
+  const getData = () => {
+    store.fetch();
+  };
+
+  $: console.log(`without_load - isFetching: ${$store.isFetching}`);
+</script>
+
+<button on:click={getData}>Fetch</button>
+
+<pre>{JSON.stringify($store, null, 2)}</pre>

packages/houdini-react/src/runtime/index.ts

@@ -14,6 +14,9 @@ export async function query(artifact: QueryArtifact, variables?: any) {
 			session: {},
 			metadata: {},
 		},
+		setFetching: () => {
+			console.log('fetching...')
+		},
 	})
 
 	return [result.result]

packages/houdini-svelte/src/runtime/stores/mutation.ts

@@ -20,10 +20,14 @@ export class MutationStore<
 
 	private store: Writable<MutationResult<_Data, _Input>>
 
+	protected setFetching(isFetching: boolean) {
+		this.store?.update((s) => ({ ...s, isFetching }))
+	}
+
 	constructor({ artifact }: { artifact: MutationArtifact }) {
 		super()
 		this.artifact = artifact
-		this.store = writable(this.nullState)
+		this.store = writable(this.initialState)
 	}
 
 	async mutate(
@@ -85,6 +89,7 @@ export class MutationStore<
 				artifact: this.artifact,
 				variables: newVariables,
 				session: await getSession(),
+				setFetching: (val) => this.setFetching(val),
 				cached: false,
 				metadata,
 				fetch,
@@ -161,7 +166,7 @@ export class MutationStore<
 		return this.store.subscribe(...args)
 	}
 
-	private get nullState() {
+	private get initialState() {
 		return {
 			data: null as _Data | null,
 			errors: null,

packages/houdini-svelte/src/runtime/stores/pagination/cursor.ts

@@ -49,9 +49,6 @@ export function cursorHandlers<_Data extends GraphQLObject, _Input>({
 		const config = await getConfig()
 		const client = await getCurrentClient()
 
-		// set the loading state to true
-		setFetching(true)
-
 		// build up the variables to pass to the query
 		const loadVariables: Record<string, any> = {
 			...(await extraVariables?.()),
@@ -69,6 +66,7 @@ export function cursorHandlers<_Data extends GraphQLObject, _Input>({
 			artifact,
 			variables: loadVariables,
 			session: await getSession(),
+			setFetching,
 			cached: false,
 			config,
 			fetch,
@@ -218,9 +216,6 @@ export function cursorHandlers<_Data extends GraphQLObject, _Input>({
 				queryVariables[artifact.refetch!.update === 'prepend' ? 'last' : 'first'] = count
 			}
 
-			// set the loading state to true
-			setFetching(true)
-
 			// send the query
 			const result = await fetch({
 				...params,

packages/houdini-svelte/src/runtime/stores/pagination/offset.ts

@@ -69,9 +69,6 @@ export function offsetHandlers<_Data extends GraphQLObject, _Input>({
 				throw missingPageSizeError('loadNextPage')
 			}
 
-			// set the loading state to true
-			setFetching(true)
-
 			// send the query
 			const { result } = await executeQuery<GraphQLObject, {}>({
 				client: await getCurrentClient(),
@@ -80,6 +77,7 @@ export function offsetHandlers<_Data extends GraphQLObject, _Input>({
 				session: await getSession(),
 				cached: false,
 				config,
+				setFetching,
 				fetch,
 				metadata,
 			})
@@ -127,9 +125,6 @@ export function offsetHandlers<_Data extends GraphQLObject, _Input>({
 				queryVariables.limit = count
 			}
 
-			// set the loading state to true
-			setFetching(true)
-
 			// send the query
 			const result = await fetch.call(this, {
 				...params,

packages/houdini-svelte/src/runtime/stores/query.ts

@@ -35,7 +35,7 @@ export class QueryStore<
 	// identify it as a query store
 	kind = CompiledQueryKind
 
-	// at its core, a query store is a writable store with extra methods{
+	// at its core, a query store is a writable store with extra methods
 	protected store: Writable<StoreState<_Data, _Input, _ExtraFields>>
 
 	// we will be reading and write the last known variables often, avoid frequent gets and updates
@@ -54,6 +54,14 @@ export class QueryStore<
 	// the string identifying the store
 	protected storeName: string
 
+	protected setFetching(isFetching: boolean) {
+		this.store?.update((s) => ({ ...s, isFetching }))
+	}
+
+	protected async currentVariables() {
+		return get(this.store).variables
+	}
+
 	constructor({ artifact, storeName, variables }: StoreConfig<_Data, _Input, QueryArtifact>) {
 		super()
 
@@ -73,8 +81,6 @@ export class QueryStore<
 	fetch(params?: QueryStoreFetchParams<_Data, _Input>): Promise<QueryResult<_Data, _Input>>
 	async fetch(args?: QueryStoreFetchParams<_Data, _Input>): Promise<QueryResult<_Data, _Input>> {
 		const config = await this.getConfig()
-		// set the cache's config
-		getCache().setConfig(config)
 
 		// validate and prepare the request context for the current environment (client vs server)
 		const { policy, params, context } = await fetchParams(this.artifact, this.storeName, args)
@@ -128,8 +134,6 @@ If this is leftovers from old versions of houdini, you can safely remove this \`
 		// we might not want to wait for the fetch to resolve
 		const fakeAwait = clientStarted && isBrowser && !params?.blocking
 
-		this.setFetching(true)
-
 		// perform the network request
 		const request = this.fetchAndCache({
 			config,
@@ -161,10 +165,6 @@ If this is leftovers from old versions of houdini, you can safely remove this \`
 		return this.artifact.name
 	}
 
-	protected async currentVariables() {
-		return get(this.store).variables
-	}
-
 	subscribe(
 		...args: Parameters<Readable<QueryResult<_Data, _Input, _ExtraFields>>['subscribe']>
 	) {
@@ -226,6 +226,7 @@ If this is leftovers from old versions of houdini, you can safely remove this \`
 		const request = await fetchQuery<_Data, _Input>({
 			...context,
 			client: await getCurrentClient(),
+			setFetching: (val) => this.setFetching(val),
 			artifact,
 			variables,
 			cached,
@@ -342,15 +343,11 @@ If this is leftovers from old versions of houdini, you can safely remove this \`
 		this.lastVariables = newVariables
 	}
 
-	protected setFetching(isFetching: boolean) {
-		this.store?.update((s) => ({ ...s, isFetching }))
-	}
-
 	private get initialState(): QueryResult<_Data, _Input> & _ExtraFields {
 		return {
 			data: null,
 			errors: null,
-			isFetching: false,
+			isFetching: true,
 			partial: false,
 			source: null,
 			variables: {} as _Input,
@@ -443,7 +440,7 @@ import type { LoadEvent } from '@sveltejs/kit';
 
 export async function load(${log.yellow('event')}: LoadEvent) {
 	return {
-		...load_MyQuery({ ${log.yellow('event')}, variables: { ... } })
+		...load_${storeName}({ ${log.yellow('event')}, variables: { ... } })
 	};
 }
 `

packages/houdini/src/runtime/lib/network.ts

@@ -173,6 +173,7 @@ export async function executeQuery<_Data extends GraphQLObject, _Input extends {
 	artifact,
 	variables,
 	session,
+	setFetching,
 	cached,
 	fetch,
 	metadata,
@@ -181,6 +182,7 @@ export async function executeQuery<_Data extends GraphQLObject, _Input extends {
 	artifact: QueryArtifact | MutationArtifact
 	variables: _Input
 	session: any
+	setFetching: (fetching: boolean) => void
 	cached: boolean
 	config: ConfigFile
 	fetch?: typeof globalThis.fetch
@@ -194,6 +196,7 @@ export async function executeQuery<_Data extends GraphQLObject, _Input extends {
 			session,
 		},
 		artifact,
+		setFetching,
 		variables,
 		cached,
 	})
@@ -211,16 +214,18 @@ export async function executeQuery<_Data extends GraphQLObject, _Input extends {
 
 export async function fetchQuery<_Data extends GraphQLObject, _Input extends {}>({
 	client,
+	context,
 	artifact,
 	variables,
+	setFetching,
 	cached = true,
 	policy,
-	context,
 }: {
 	client: HoudiniClient
 	context: FetchContext
 	artifact: QueryArtifact | MutationArtifact
 	variables: _Input
+	setFetching: (fetching: boolean) => void
 	cached?: boolean
 	policy?: CachePolicy
 }): Promise<FetchQueryResult<_Data>> {
@@ -280,6 +285,9 @@ export async function fetchQuery<_Data extends GraphQLObject, _Input extends {}>
 		cache._internal_unstable.collectGarbage()
 	}, 0)
 
+	// tell everyone that we are fetching if the function is defined
+	setFetching(true)
+
 	// the request must be resolved against the network
 	const result = await client.sendRequest<_Data>(context, {
 		text: artifact.raw,