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.

Sign up for GitHub

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

Jump to bottom

[legacy-framework] Fix dehydrateState does not apply server side props #2617

Merged
merged 3 commits into from
Aug 11, 2021
Merged

[legacy-framework] Fix dehydrateState does not apply server side props #2617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
14d91c1
Fixes dehydrate state SSR
Jul 31, 2021
90cdd1a
rm file
Jul 31, 2021
98bdcf8
Deserialize dehydratedState
Skn0tt Aug 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions packages/core/src/blitz-app-root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,14 @@ export function withBlitzAppRoot(UserAppRoot: React.ComponentType<any>) {
document.documentElement.classList.add("blitz-first-render-complete")
}, [])

const dehydratedState = props.pageProps.dehydratedState
? SuperJSON.deserialize(props.pageProps.dehydratedState)
: undefined
let {dehydratedState, _superjson} = props.pageProps
if (dehydratedState && _superjson) {
const deserializedProps = SuperJSON.deserialize({
json: {dehydratedState},
meta: _superjson,
}) as {dehydratedState: any}
dehydratedState = deserializedProps?.dehydratedState
}

return (
<BlitzProvider dehydratedState={dehydratedState}>
Expand Down
2 changes: 1 addition & 1 deletion test/integration/queries/pages/dehydrated-state.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {Suspense} from "react"

export const getServerSideProps: GetServerSideProps = async (ctx) => {
const queryClient = new QueryClient()
const queryKey = getQueryKey(getDate, null)
const queryKey = getQueryKey(getDate, undefined)
Copy link
Member

Choose a reason for hiding this comment

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

Good catch spotting the wrong key argument! Maybe we can ship an abstraction with Blitz that makes these kinds of errors harder:

export const getServerSideProps = async ctx => {
  return {
    props: {
      ...(await prefetchedQueries([ getDate, undefined ], ...))
    }
  }
}
async function prefetchedQueries(...pairs) {
  const queryClient = new QueryClient()
  for (const [query, arg] of pairs) {
   const key = getQuerykey(query, arg)
   await queryClient.prefetchQuery(key, () => invokeWithMiddleware(query, arg, ctx))
  }
  return { dehydratedState: dehydrate(queryClient) }
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting suggestion, I like it. It took me a while to figure it out and there was a small piece in the react-query docs that made me realize there was a mismatch.

Anyway we can make it easier for users to form the right code I am in support of :)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I knew from the start we needed to make this better.

How about this?

import {prefetchQuery} from 'blitz'

export const getServerSideProps = async ctx => {

  await prefetchQuery(getData, undefined)

  return { props: {} }
}

And then we automatically add new QueryClient(), dehydrate etc via a babel plugin.

Copy link
Collaborator Author

@clgeoio clgeoio Aug 5, 2021 •
edited

Choose a reason for hiding this comment

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

A small suggestion would be to require that dehydratedState be passed into props: {} to avoid the edge case of a user declaring a prop dehydratedState and having overridden without them knowing by babel.
Perhaps we could rename the prop to prefetchedQuery for clarity and then transform in blitz-root?

export const getServerSideProps = async ctx => {
  const dehydratedState = await prefetchQuery(getData, undefined)
  return { props: {
    dehydratedState
  } }
}

Copy link
Member

Choose a reason for hiding this comment

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

The babel plugin can throw a helpful error or something in that case, right? I think having dehydratedState exposed is exposing unnecessary implementation details.

await queryClient.prefetchQuery(queryKey, () => invokeWithMiddleware(getDate, undefined, ctx))

return {
Expand Down
1 change: 0 additions & 1 deletion test/integration/queries/test/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ describe("Queries", () => {
describe("DehydratedState", () => {
it("should work", async () => {
const browser = await webdriver(context.appPort, "/dehydrated-state")
await browser.waitForElementByCss("#content")
Copy link
Member

Choose a reason for hiding this comment

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

I definitely see why this line is problematic for the test.

let text = await browser.elementByCss("#content").text()
expect(text).toMatch(/date is Date: true/)
if (browser) await browser.close()
Expand Down