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

[clgeoio]

Closes: blitz-js/legacy-framework#54

What are the changes and their implications?

The query that was setup in the dehydrated state did not have matching params (null vs undefined). Additionally, the SuperJSON deserialisation seemed to be causing some issues with the cached query being used on the SSR.

However, this change looks to undo: #2512

Bug Checklist

• Integration test added (see test docs if needed)

Feature Checklist

• Integration test added (see test docs if needed)
• Documentation added/updated (submit PR to blitzjs.com repo)

[Skn0tt]

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

[Skn0tt]

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) }
}

[clgeoio]

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 :)

[flybayer]

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.

[clgeoio]

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
  } }
}

[flybayer]

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

[Skn0tt]

It's interesting to me that removing the deserialisation helps in this case, since babel-plugin-superjson-next should be serializing dehydratedState:

blitz/packages/babel-preset/src/index.ts

Line 16 in a28216d

require('babel-plugin-superjson-next'),

🤔

[Skn0tt]

I pushed an update in 98bdcf8, which should fix the behaviour while retaining SuperJSON support. The problem basically was that SuperJSON.deserialize was a totally wrong call - see the 98bdcf8's commit message for more context.

[clgeoio]

Awesome, thanks for making that change!

I think I will add a test for some of the more complex types, like Map

[clgeoio]

Interestly @Skn0tt, if I swap out getDate with getComplexTypes and return something like:

export default async function getComplexTypes() {
  return {
    date: new Date(0),
    map: new Map().set("foo", "bar"),
    obj: {
      one: 1,
      child: {
        two: 2,
      },
    },
  }
}

When map: new Map() is in the returned object, the test fails:

 NoSuchElementError: no such element: Unable to locate element: {"method":"css selector","selector":"#content"}
      (Session info: headless chrome=92.0.4515.107)

      77 |     it("should work", async () => {
      78 |       const browser = await webdriver(context.appPort, "/dehydrated-state")
    > 79 |       const content = await browser.elementByCss("#content")

But when the map key is omitted from the return object, the test passes.

Any ideas here, should this be supported?

[Skn0tt]

Yes, it should definitely be supported. That's some weird behaviour! Will look into it.

[clgeoio]

Thank you @Skn0tt.
Let me know if I can lend a hand.
I'll park this PR until you've had a look 👍

[flybayer]

@Skn0tt what's the status on this PR?

[Skn0tt]

@Skn0tt what's the status on this PR?

looking at it now, let's hope I find something :)

[Skn0tt]

@flybayer @clgeoio the failure that @clgeoio found seems unrelated to this PR. Something seems to be mutating the SuperJSON before it's inlined into the __NEXT_PROPS 🤔 I'll push a reproduction test to a separate PR. This PR should be fine, and once the tests are re-run (think they failed for network connection reasons) it looks good to me.