-
Notifications
You must be signed in to change notification settings - Fork 27k
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
Apollo example: avoid double render in browser #4734
Conversation
Thanks for your work however I've noticed a issue on the client when applying this PR. Without the PR it'll wait for the result of the query before rendering the Page/changing the route. When applying the PR it'll change the route before loading the query. https://streamable.com/t6dit Maybe there is something wrong with my implementation. The relevant code of the page: class Series extends React.Component {
static getInitialProps({ query: { id } }) {
// This apply the query.id to this.props.id
return { id };
}
render() {
const { classes, id } = this.props;
return (
<Query
query={QueryGetSeries}
variables={{
tvdbId: id,
}}
>
{({ loading, error, data }) => {
if (loading && !data) return 'Loading...';
if (error) return `Error! ${error.message}`;
if (!data || !data.getSeries) {
return <Error statusCode={404} />; // Here it throws the 404 page that you can see in the video with this PR code
}
return (
<Layout box>
<PageTitle title={data.getSeries.name} />
</Layout>
);
}}
</Query>
);
}
} The ID prop is coming from the link: <Link href={`/series?id=${id}`} as={`/s/${id}`}>...</Link> However I can confirm that it triggers render() only once (instead two times). |
"When applying the PR it'll change the route before loading the query", yes, and it is good, you go to a new page, it renders a Query children with loading state, makes a request and renders children with data. I consider this as a basic flow for apps with ApolloClient. You can make some modifications to avoid displaying loading state, but only if you want to. This can be achieved by various prefetching techniques, one of them is a fake render in getInitialProps to fill the cache for the real render, but it will work only if you change pages, if you have several tabs on the same page, it won't work, you will get a loading state. One more technique is described in apollo docs. About your implementation. It is strange, seems like it should work, may be somehow you are getting a not empty data in loading state, could you publish the whole app or log all Query's children props? |
The weird thing is that |
It seems that cache contains the requested data. |
Oh looks like After changing it to this it'll actually displays a - if (loading && !data) return 'Loading...';
+ if (loading) return 'Loading...'; But my approach with getInitialProps() doesn't work anymore. It'll change the route instant and shows the loading state. Anyway it's late and thanks for your input. |
If you want to prefetch data without showing loading state you can always use fake render on both server and client side. |
@NikitaVlaznev One thing I noticed with this change is that you can no longer prefetch data. I use this example to prefetch data but it requires being able to run |
I suppose you can fake render on the client with export const prefetch = async href => {
// if we're running server side do nothing
if (typeof window === 'undefined') return
const url = typeof href !== 'string' ? format(href) : href
const { pathname } = window.location
const parsedHref = resolve(pathname, url)
const { query } = typeof href !== 'string' ? href : parse(url, true)
const Component = await Router.prefetch(parsedHref)
// if Component exists and has getInitialProps
// fetch the component props (the component should save it in cache)
if (Component && Component.getInitialProps) {
const ctx = { pathname: href, query, isVirtualCall: true }
const composedInitialProps = await Component.getInitialProps(ctx)
await getDataFromTree(
<Component ctx={ctx} {...composedInitialProps} />,
{
router: {
asPath: ctx.asPath,
pathname: ctx.pathname,
query: ctx.query
}
}
)
}
} |
Apollo's getDataFromTree is supposed to be called during the server side rendering.
Being called in browser it fires an unnecessary fake render process and blocks components from rendering with loading=true.
Also there was a mistake in this code:
Apollo component is not rendered by getDataFromTree actually, it renders the App directly, thus props.apolloClient will always be undefined.
This example was discussed here: #387.