Skip to content
This repository has been archived by the owner on Mar 8, 2020. It is now read-only.

Add better support for _app.js. Move types to separate module. #38

Merged
merged 3 commits into from
Feb 21, 2020

Conversation

parkerziegler
Copy link
Contributor

Fix #36

This PR does a few things, most importantly fixing how next-urql returns pageProps when wrapping an _app.js component. We weren't handling this properly previously when running the prepass step. _app.js has a special props signature.

const App = ({ Component, pageProps }) =>

in contrast to a regular Page component, which just consumes its props directly. The extra pageProps layer is something we needed to take into account in our HoC. Moreover, we also need to ensure we use the proper ctx signature for _app.js vs. Page components, as these structures differ as well. This PR adds an isApp check inside getInitialProps such that we attach urqlClient to the correct ctx object.

In order to make a lot of this work, I loosened the types a bit by moving away from our generics. This is fine, since the HoC doesn't really care about any props coming in from the outside (it just forwards them along), and users can just specify their component's prop types in userland.

Finally, I moved our types into a separate ./types module to make them easier to read ✅


return (
<Provider value={client}>
<Page urqlClient={client} {...(rest as T & IP)} />
<AppOrPage urqlClient={client} {...rest} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went for naming this AppOrPage – a much better description of what this can be!

/>,
);
const props = { ...pageProps, urqlClient };
const appTreeProps = isApp ? props : { pageProps: props };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

_app and Page components have different prop signatures, so we need to use the correct ones depending on what AppTree actually is.

Copy link

@kitten kitten left a comment

Choose a reason for hiding this comment

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

That makes a lot of sense! Happy we can increase consistency here to smooth over the Next.js API quirks 🙌

@nghiepdev
Copy link

💯 💯 💯

@parkerziegler parkerziegler merged commit 90261db into master Feb 21, 2020
@parkerziegler parkerziegler deleted the task/support-_app-pageProps branch February 21, 2020 00:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: pageProps doesn't pass into Component when custom _app.js
3 participants