fix(v5): type queryResult in Cells

[jtoar]

Fixes #7024 (comment). @Tobbe works in VS Code with yarn rwfw going, but do you think it's this simple?

[replay-io]

15 replays were recorded for 7c94b4f.

0 Failed

15 Passed

• useAuth hook, auth redirects checks
• Check that a specific blog post is prerendered
• Check that about is prerendered
• Check that homepage is prerendered
• Check that meta-tags are rendering the correct dynamic data
• Check that you can navigate from home page to specific blog post
• Waterfall prerendering (nested cells)
• RBAC: Admin user should be able to delete contacts
• RBAC: Should not be able to delete contact as non-admin user
• Smoke test with dev server
• Smoke test with rw serve
• Loads Cell mocks when Cell is nested in another story
• Loads Cell Stories
• Loads MDX Stories
• Mocks current user, and updates UI while dev server is running

View test run on Replay ↗︎

[Tobbe]

do you think it's this simple?

Do we need to make the same change for Empty, Loading and Failure too?

[jtoar]

@Tobbe ah yeah great catch, just pushed those changes up

[Tobbe]

Sorry I didn't catch this earlier, but I wonder if Partial<> isn't in the wrong spot now. I could be wrong here, so please validate my claims. But I think what you're saying now is that queryResult might or might not be present. And updating might or might not be present.

Basically this:

type CellSuccessProps = {
  queryResult?: { ... }
  updating?: boolean
}

Whereas what we had before was everything "inside" queryResult being optional.

So, maybe what we need is something like

type CellSuccessProps = {
  queryResult: Partial<{ ... }>
  updating: boolean
}

I removed the ? after both queryResult and updating. I'm not sure if for example updating can be undefined, and if so, how React will treat <Success updating={undefined}>. If it can be undefined, and react then doesn't include it in the props for <Success>, then we need to add the ?.

[jtoar]

Yeah I thought about this briefly. We had Partial wrapping the whole type before so pretty much everything except for the data comes back as being possible undefined:

Including updating:

It makes sense to make queryResult's contents Partial but not the prop itself, but making updating not possibly undefined would be a change I haven't considered yet. Open to anything though I just haven't through it through, was just trying to fix the bug

[jtoar]

With the typing the way it is I have to pass queryResult like this in the type test now; maybe it's not a big deal since it'll be passed by the Cell to Success, etc automatically: 6404619

[Tobbe]

Open to anything though I just haven't through it through, was just trying to fix the bug

You're right. Let's focus on the actual bug! 🙂

[Tobbe]

With the typing the way it is I have to pass queryResult like this in the type test now

I think that's correct. The "input" to a cell should/will have queryResult.