Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(solid-query): ssr fixes #5093

Merged
merged 10 commits into from
Apr 3, 2023
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
2 changes: 1 addition & 1 deletion examples/solid/basic-graphql-request/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"@tanstack/solid-query": "^4.3.9",
"graphql": "^16.6.0",
"graphql-request": "^5.0.0",
"solid-js": "^1.6.2"
"solid-js": "^1.6.13"
},
"devDependencies": {
"typescript": "4.7.4",
Expand Down
2 changes: 1 addition & 1 deletion examples/solid/basic-typescript/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"license": "MIT",
"dependencies": {
"@tanstack/solid-query": "^4.3.9",
"solid-js": "^1.6.2"
"solid-js": "^1.6.13"
},
"devDependencies": {
"typescript": "4.7.4",
Expand Down
2 changes: 1 addition & 1 deletion examples/solid/default-query-function/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"license": "MIT",
"dependencies": {
"@tanstack/solid-query": "^4.3.9",
"solid-js": "^1.6.2"
"solid-js": "^1.6.13"
},
"devDependencies": {
"typescript": "4.7.4",
Expand Down
2 changes: 1 addition & 1 deletion examples/solid/simple/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"license": "MIT",
"dependencies": {
"@tanstack/solid-query": "^4.3.9",
"solid-js": "^1.6.2"
"solid-js": "^1.6.13"
},
"devDependencies": {
"@tanstack/eslint-plugin-query": "^4.13.0",
Expand Down
12 changes: 6 additions & 6 deletions examples/solid/solid-start-streaming/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
"esbuild": "^0.14.54",
"postcss": "^8.4.18",
"solid-start-node": "^0.2.0",
"typescript": "^4.8.4",
"vite": "^3.1.8"
"typescript": "^4.9.4",
"vite": "^4.1.4"
},
"dependencies": {
"@tanstack/solid-query": "^4.3.9",
"@tanstack/solid-query": "^5.0.0-alpha.3",
"@solidjs/meta": "^0.28.2",
"@solidjs/router": "^0.6.0",
"solid-js": "^1.6.9",
"solid-start": "^0.2.15",
"@solidjs/router": "^0.7.0",
"solid-js": "^1.6.13",
"solid-start": "^0.2.23",
"undici": "^5.11.0"
},
"engines": {
Expand Down
10 changes: 10 additions & 0 deletions examples/solid/solid-start-streaming/src/root.css
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,13 @@ p {
font-weight: bold;
flex-grow: 1;
}

.example--table {
padding: 0.5rem;
}

.example--table th,
.example--table td {
padding: 4px 12px;
white-space: nowrap;
}
3 changes: 2 additions & 1 deletion examples/solid/solid-start-streaming/src/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export default function Root() {
const queryClient = new QueryClient({
defaultOptions: {
queries: {
retry: 0,
retry: false,
},
},
})
Expand All @@ -43,6 +43,7 @@ export default function Root() {
<A href="/deferred">Deferred</A>
<A href="/mixed">Mixed</A>
<A href="/with-error">With Error</A>
<A href="/hydration">Hydration</A>

<Routes>
<FileRoutes />
Expand Down
99 changes: 99 additions & 0 deletions examples/solid/solid-start-streaming/src/routes/hydration.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import type { CreateQueryResult } from '@tanstack/solid-query'
import { createQuery } from '@tanstack/solid-query'
import { createSignal, Suspense } from 'solid-js'
import { fetchUser } from '~/utils/api'
import { NoHydration } from 'solid-js/web'
import { Title } from 'solid-start'

export default function Hydration() {
const query = createQuery(() => ({
queryKey: ['user'],
queryFn: () => fetchUser({ sleep: 500 }),
deferStream: true,
}))

const [initialQueryState] = createSignal(JSON.parse(JSON.stringify(query)))

return (
<main>
<Title>Solid Query - Hydration</Title>

<h1>Solid Query - Hydration Example</h1>

<div class="description">
<p>
Lists the query state as seen on the server, initial render on the
client (right after hydration), and current client value. Ideally, if
SSR is setup correctly, these values are exactly the same in all three
contexts.
</p>
</div>

<button onClick={() => query.refetch()}>Refetch</button>

<table class="example example--table">
<thead>
<tr>
<th>Context</th>
<th>data.name</th>
<th>isFetching</th>
<th>isFetched</th>
<th>isPending</th>
<th>isRefetching</th>
<th>isLoading</th>
<th>isStale</th>
<th>isSuccess</th>
<th>isError</th>
<th>error</th>
<th>fetchStatus</th>
<th>dataUpdatedAt</th>
</tr>
</thead>

<tbody>
<Suspense>
<NoHydration>
<QueryStateRow context="server" query={query} />
</NoHydration>

<QueryStateRow
context="client (initial render)"
query={initialQueryState()!}
/>

<QueryStateRow context="client" query={query} />
</Suspense>
</tbody>
</table>
</main>
)
}

type QueryState = CreateQueryResult<
{
id: string
name: string
queryTime: number
},
Error
>

const QueryStateRow = (props: { context: string; query: QueryState }) => {
return (
<tr>
<td>{props.context}</td>
<td>{props.query.data?.name}</td>
<td>{String(props.query.isFetching)}</td>
<td>{String(props.query.isFetched)}</td>
<td>{String(props.query.isPending)}</td>
<td>{String(props.query.isRefetching)}</td>
<td>{String(props.query.isLoading)}</td>
<td>{String(props.query.isStale)}</td>
<td>{String(props.query.isSuccess)}</td>
<td>{String(props.query.isError)}</td>
<td>{String(props.query.error)}</td>
<td>{String(props.query.fetchStatus)}</td>
<td>{String(props.query.dataUpdatedAt)}</td>
</tr>
)
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
"rollup-plugin-visualizer": "^5.9.0",
"rollup-preset-solid": "^1.4.0",
"semver": "^7.3.8",
"solid-js": "^1.5.7",
"solid-js": "^1.6.13",
"solid-testing-library": "^0.3.0",
"stream-to-array": "^2.3.0",
"ts-node": "^10.9.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/solid-query/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"@tanstack/query-core": "workspace:*"
},
"peerDependencies": {
"solid-js": "^1.6.2"
"solid-js": "^1.6.13"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumped for this fix -> solidjs/solid@60f8624

},
"peerDependenciesMeta": {}
}
40 changes: 27 additions & 13 deletions packages/solid-query/src/createBaseQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
createResource,
on,
onCleanup,
onMount,
} from 'solid-js'
import { createStore, unwrap } from 'solid-js/store'
import { useQueryClient } from './QueryClientProvider'
Expand Down Expand Up @@ -64,7 +63,20 @@ export function createBaseQuery<
) => {
return observer.subscribe((result) => {
notifyManager.batchCalls(() => {
const unwrappedResult = { ...unwrap(result) }
const query = observer.getCurrentQuery()
const unwrappedResult = {
...unwrap(result),

// hydrate() expects a QueryState object, which is similar but not
// quite the same as a QueryObserverResult object. Thus, for now, we're
// copying over the missing properties from state in order to support hydration
dataUpdateCount: query.state.dataUpdateCount,
fetchFailureCount: query.state.fetchFailureCount,
fetchFailureReason: query.state.fetchFailureReason,
fetchMeta: query.state.fetchMeta,
isInvalidated: query.state.isInvalidated,
Comment on lines +70 to +77
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does indeed fix the isFetched hydration issue. It's hacky, but works for now until there's a better solution 🤷‍♂️.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates! Yeah this feels like a fine tradeoff for now!

}

if (unwrappedResult.isError) {
if (process.env['NODE_ENV'] === 'development') {
console.error(unwrappedResult.error)
Expand Down Expand Up @@ -178,13 +190,17 @@ export function createBaseQuery<
}
})

onMount(() => {
observer.setOptions(defaultedOptions, { listeners: false })
})
Comment on lines -181 to -183
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell, this isn't doing anything. Removed to simplify and reduce the number of calls to queryObserver.createResult. LMK if there's a reason for this and I can revert np!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have to test more but this is from the first iteration of creating the adapter. I dont think we would need for now


createComputed(() => {
observer.setOptions(client().defaultQueryOptions(options()))
})
createComputed(
on(
() => client().defaultQueryOptions(options()),
() => observer.setOptions(client().defaultQueryOptions(options())),
{
// Defer because we don't need to trigger on first render
// This only cares about changes to options after the observer is created
defer: true,
},
),
)

createComputed(
on(
Expand All @@ -209,10 +225,8 @@ export function createBaseQuery<
target: QueryObserverResult<TData, TError>,
prop: keyof QueryObserverResult<TData, TError>,
): any {
if (prop === 'data') {
return queryResource()?.data
}
return Reflect.get(target, prop)
const val = queryResource()?.[prop]
return val !== undefined ? val : Reflect.get(target, prop)
},
}

Expand Down
Loading