Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/fix-paginated-fragment-at-loading.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'houdini': patch
'houdini-svelte': patch
'houdini-react': patch
---

A paginated fragment spread on an `@loading` query no longer fires a `node(id: PendingValue)` request while its parent is still loading. The pagination handlers no-op until the parent entity resolves, so the fragment works without an `if (!loading)` guard.
7 changes: 7 additions & 0 deletions .changeset/react-loading-hydration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'houdini-core': patch
'houdini': patch
'houdini-react': patch
---

Fix React `@loading` pages failing to hydrate on the client, which left them non-interactive and prevented paginated fragments spread on an `@loading` query from rendering or paginating once the data resolved.
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

Canonical example: `packages/houdini-core/plugin/validate_test.go`. TypeScript test helpers: `testConfig()` / `testConfigFile()` in `packages/houdini/src/test/index.ts`.

**Updating a golden artifact**: the artifact table tests (e.g. `selection_*_test.go`) compare the whole generated artifact with `require.Equal`, so when a codegen change shifts the expected output, do NOT hand-edit the golden surgically — that's error-prone and has caused confusion. Instead: replace the entire expected value for that case with a placeholder (`tests.Dedent(\`PLACEHOLDER\`)`), run the test, copy the `actual:` string from the failure into place, then re-run and eyeball the diff to confirm the shape changed only the way you intended. Two tips: the failure prints `actual` already Go-escaped, so it can be pasted as a plain double-quoted string (no `Dedent`/backtick juggling needed); and when a change ripples across several cases, capture each `actual:` and splice it in (anchor on the case's unique hash if keys repeat) rather than editing by hand.

## React route typing

Per-route TypeScript typing (which `params` and `search` a route accepts, the `RouteHrefs` union, scalar resolution) lives in **one** place: `packages/houdini-react/runtime/routes.ts`. It derives everything from the generated manifest's shape via `typeof rawManifest`, and exports `RouteHrefs`, `ParamsForRoute<H>`, `SearchForRoute<H>`, `NavTarget<H>`, and `Goto`.
Expand Down
1 change: 1 addition & 0 deletions e2e/kit/src/lib/utils/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const routes = {
fragment_masking_partial: '/fragment-masking-partial',
conditional_fragment_spread: '/conditional-fragment-spread',
loading_state: '/loading-state',
paginated_fragment_at_loading: '/bug/paginated-fragment-at-loading',
required_field: '/required-field',
Cache_Refresh: '/cache/refresh',
Cache_Refetch: '/cache/refetch',
Expand Down
23 changes: 23 additions & 0 deletions e2e/kit/src/routes/bug/paginated-fragment-at-loading/+page.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<script lang="ts">
import { PendingValue } from '$houdini'
import type { PageData } from './$types'
import Friends from './Friends.svelte'

export let data: PageData

$: ({ PaginatedFragmentAtLoading: store } = data)
</script>

{#if $store.data}
<div id="name">
{#if $store.data.user.name === PendingValue}
loading...
{:else}
{$store.data.user.name}
{/if}
</div>

<!-- rendered unguarded: the child spreads a @paginate fragment and must survive the
parent's @loading frame (issue #1408, where the if (!loading) guard was required) -->
<Friends user={$store.data.user} />
{/if}
21 changes: 21 additions & 0 deletions e2e/kit/src/routes/bug/paginated-fragment-at-loading/+page.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import type { PageLoad } from './$types'
import { graphql } from '$houdini'

// issue #1408: a @paginate fragment spread on a document-level @loading query. the fragment
// itself carries no @loading; it relies on the query-level @loading cascading into the spread.
const store = graphql(`
query PaginatedFragmentAtLoading @loading {
user(id: "1", snapshot: "paginated-fragment-at-loading", delay: 2000) {
name
...PaginatedFragmentAtLoading_Friends
}
}
`)

export const load: PageLoad = async (event) => {
await store.fetch({ event })

return {
PaginatedFragmentAtLoading: store,
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<script lang="ts">
import { graphql, paginatedFragment, type PaginatedFragmentAtLoading_Friends } from '$houdini'

export let user: PaginatedFragmentAtLoading_Friends

$: store = paginatedFragment(
user,
graphql(`
fragment PaginatedFragmentAtLoading_Friends on User {
friendsConnection(first: 2) @paginate {
edges {
node {
name
}
}
}
}
`)
)
</script>

{#if $store.data}
<div id="result">
{$store.data.friendsConnection.edges.map(({ node }) => node?.name).join(', ')}
</div>
{/if}

<!-- rendered unconditionally so it can be clicked during the parent's @loading frame: the
handler must no-op then rather than fire node(id: PendingValue) (issue #1408) -->
<button id="next" on:click={() => store.loadNextPage()}>next</button>
34 changes: 34 additions & 0 deletions e2e/kit/src/routes/bug/paginated-fragment-at-loading/spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { test } from '@playwright/test'
import { routes } from '../../../lib/utils/routes.js'
import {
clientSideNavigation,
expect_0_gql,
expect_1_gql,
expect_to_be,
goto,
} from '../../../lib/utils/testsHelper.js'

test.describe('paginated fragment under a document-level @loading query', () => {
// issue #1408: a @paginate fragment spread on an @loading query. The child is rendered
// unguarded, so its pagination handlers can be invoked while the parent entity is still a
// PendingValue placeholder. They must no-op then (instead of firing node(id: PendingValue))
// and resume working once @loading resolves — all without an `if (!loading)` guard.
test('no-ops while loading, renders and paginates after', async ({ page }) => {
// must navigate client-side to actually see the @loading frame
await goto(page, routes.Home)
await clientSideNavigation(page, routes.paginated_fragment_at_loading)

// we're in the loading frame
await expect_to_be(page, 'loading...', '#name')

// clicking next while the parent is still loading must not fire a request
await expect_0_gql(page, 'button[id=next]')

// once @loading resolves the friends render
await expect_to_be(page, 'Bruce Willis, Samuel Jackson', '#result')

// and pagination works from there
await expect_1_gql(page, 'button[id=next]')
await expect_to_be(page, 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks', '#result')
})
})
5 changes: 5 additions & 0 deletions e2e/react/src/routes/loading-interactive/+page.gql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
query LoadingInteractiveQuery @loading {
user(id: "1", snapshot: "loading-interactive", delay: 400) {
name
}
}
25 changes: 25 additions & 0 deletions e2e/react/src/routes/loading-interactive/+page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { isPending } from '$houdini'
import * as React from 'react'

import type { PageProps } from './$types'

// The simplest possible reproduction of issue #1408's underlying cause: a page whose
// query is marked @loading renders its loading frame inside a streaming Suspense
// boundary. Once the data resolves the real markup is swapped in by the server stream,
// but the client must still hydrate it so the page is interactive. This button proves
// that: if hydration never commits, the counter stays at 0 no matter how many times
// it is clicked.
export default function ({ LoadingInteractiveQuery }: PageProps) {
const user = LoadingInteractiveQuery.user
const [count, setCount] = React.useState(0)

return (
<>
<div id="name">{isPending(user.name) ? 'loading' : user.name}</div>
<div id="count">{count}</div>
<button id="increment" onClick={() => setCount((c) => c + 1)}>
increment
</button>
</>
)
}
23 changes: 23 additions & 0 deletions e2e/react/src/routes/loading-interactive/test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { test } from '@playwright/test'
import { routes } from '~/utils/routes.js'
import { expect_to_be, goto } from '~/utils/testsHelper.js'

test.describe('@loading page hydration', () => {
// issue #1408: a page whose query is marked @loading streams its loading frame and then
// has the resolved markup swapped in by the server stream. The client must still hydrate
// the page so it is interactive. A button that increments a counter is the minimal proof:
// if hydration never commits, clicking it does nothing.
test('the page is interactive after the loading state resolves', async ({ page }) => {
await goto(page, routes.loading_interactive)

// the real data lands once the @loading query resolves
await expect_to_be(page, 'Bruce Willis', '#name')

// clicking the button must update the counter, which only happens if React hydrated
await page.click('button[id=increment]')
await expect_to_be(page, '1', '#count')

await page.click('button[id=increment]')
await expect_to_be(page, '2', '#count')
})
})
6 changes: 6 additions & 0 deletions e2e/react/src/routes/loading-paginated-fragment/+page.gql
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
query LoadingFragmentQuery @loading {
user(id: "1", snapshot: "loading-paginated-fragment", delay: 2000) {
name
...LoadingFragmentList
}
}
54 changes: 54 additions & 0 deletions e2e/react/src/routes/loading-paginated-fragment/+page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { graphql, isPending, useFragmentHandle } from '$houdini'

import type { PageProps } from './$types'

const fragment = graphql(`
fragment LoadingFragmentList on User {
usersConnectionSnapshot(snapshot: "loading-paginated-fragment", first: 2) @paginate {
edges {
node {
name
}
}
pageInfo {
hasNextPage
endCursor
}
}
}
`)

// Rendered as a child so it mounts *during* the parent query's @loading frame
// (no if-guard). This is the exact scenario from issue #1408.
function FriendsList({ user }: { user: any }) {
const handle = useFragmentHandle(user, fragment)

const edges = handle.data?.usersConnectionSnapshot?.edges
const names = Array.isArray(edges)
? edges
.map(({ node }) => node?.name)
.filter((name) => typeof name === 'string')
.join(', ')
: ''

return (
<>
<div id="result">{names}</div>
<div id="pageInfo">{JSON.stringify(handle.pageInfo)}</div>
<button id="next" onClick={() => handle.loadNext()}>
next
</button>
</>
)
}

export default function ({ LoadingFragmentQuery }: PageProps) {
const user = LoadingFragmentQuery.user

return (
<>
<div id="name">{isPending(user.name) ? 'loading' : user.name}</div>
<FriendsList user={user} />
</>
)
}
48 changes: 48 additions & 0 deletions e2e/react/src/routes/loading-paginated-fragment/test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { test } from '@playwright/test'
import { routes } from '~/utils/routes.js'
import {
clientSideNavigation,
expect_0_gql,
expect_1_gql,
expect_to_be,
goto,
} from '~/utils/testsHelper.js'

test.describe('paginated fragment under an @loading query', () => {
// issue #1408: a paginated fragment spread on an @loading query mounts during the
// loading frame. once the parent query resolves the list should appear and paginate,
// without the user having to guard the component behind an `if (!loading)` check.
test('renders and paginates after loading resolves (no guard)', async ({ page }) => {
await goto(page, routes.loading_paginated_fragment)

// first page arrives with the parent query once @loading resolves
await expect_to_be(page, 'Bruce Willis, Samuel Jackson')

// and pagination keeps working from there
await expect_1_gql(page, 'button[id=next]')
await expect_to_be(page, 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks')
})

// the pagination handlers must no-op while the parent entity is still a PendingValue
// placeholder, instead of firing a node(id: PendingValue) request. the loading frame is
// only observable via a client-side navigation (a direct load streams until resolved).
test('paginating during the loading frame is a no-op', async ({ page }) => {
await goto(page, '/')
await clientSideNavigation(page, routes.loading_paginated_fragment)

// we're in the loading frame
await expect_to_be(page, 'loading', '#name')

// clicking next while the parent is still loading must not fire a request
await expect_0_gql(page, 'button[id=next]')

// once @loading resolves the friends render (the #result node is present but empty
// during loading, so wait for the data before asserting) and pagination works
await page.waitForFunction(() =>
document.querySelector('#result')?.textContent?.includes('Bruce')
)
await expect_to_be(page, 'Bruce Willis, Samuel Jackson', '#result')
await expect_1_gql(page, 'button[id=next]')
await expect_to_be(page, 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks', '#result')
})
})
2 changes: 2 additions & 0 deletions e2e/react/src/utils/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export const routes = {
pagination_fragment_cursor_forwards_singlepage: '/pagination/fragment/connection-forwards-singlepage',
pagination_fragment_cursor_backwards_singlepage: '/pagination/fragment/connection-backwards-singlepage',
pagination_query_offset_variable: '/pagination/query/offset-variable/2',
loading_paginated_fragment: '/loading-paginated-fragment',
loading_interactive: '/loading-interactive',
refetchable_fragment: '/refetchable-fragment',
refetchable_fragment_custom: '/refetchable-fragment-custom',
optimistic_keys: '/optimistic-keys',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,12 @@ func stringifySelection(
loadingTypes := []string{}

for _, selection := range selections {
hasLoading := false
// inherit the cascading loading state (a document-level @loading, or a parent
// field's @loading(cascade: true)) so that fragment spreads participate in the
// loading state just like fields do. without this a spread under a global @loading
// is omitted from the loading-state selection entirely. mirrors the field path,
// which seeds hasLoading from forceLoading.
hasLoading := forceLoading
for _, directive := range selection.Directives {
switch directive.Name {
case graphql.LoadingDirective:
Expand Down
Loading
Loading