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.

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

Improve SSR performance for nested queries satisfied by cache #191

Closed
abroadwin opened this issue Nov 12, 2019 · 2 comments
Closed

Improve SSR performance for nested queries satisfied by cache #191

abroadwin opened this issue Nov 12, 2019 · 2 comments

Comments

@abroadwin
Copy link

At the moment nested queries that depend on each other result in multiple re-renders in SSR, even if the data to satisfy every level is already in the cache; this means that even with a pre-populated cache the render time scales with the number of nested queries multiplied by the general complexity of the page.

An example case is nested comments on a page that has already pre-populated the cache for every comment. I was seeing terrible performance in SSR, taking up to several seconds to render a page with ~40 comments, nested up to 10 layers deep in some cases. A page with thousands of comments would be completely untenable.

Despite cache-first queries and all of the data being in the cache each hook appears to be resolved asynchronously on the first pass, with loading true. Then it resolves to the data already in the cache, re-renders and repeats the process the next level down.

I have worked around this performance issue by adding a new wrapper component; it accesses the cache directly using client.readQuery; if the data is returned, it renders the presentation component with that data. If it catches an error, it falls back to the standard container component that uses useQuery. This successfully reduced SSR time for the page from several seconds to hundreds of milliseconds, with significantly fewer rendering passes.

This workaround is not ideal, both due to added complexity and the sacrifice of other options provided by useQuery.

My suggestion is that useQuery try to synchronously read from the cache first, and only fall back to async/loading behavior if the query is not satisfied by the cache.

@abroadwin
Copy link
Author

abroadwin commented Nov 12, 2019

A slightly improved workaround:

import { QueryHookOptions, useQuery, useApolloClient } from '@apollo/react-hooks';
import { OperationVariables, QueryResult } from '@apollo/react-common';
import { DocumentNode } from 'graphql';
import { useMemo } from 'react';

export default function useCachedQuery<TData = any, TVariables = OperationVariables>(
	query: DocumentNode,
	options?: QueryHookOptions<TData, TVariables>
): QueryResult<TData, TVariables> {
	const client = useApolloClient();

	let data: any;

	try {
		let variables;
		if (options) variables = options.variables;
		data = client.readQuery({ query, variables });
	} catch (err) {}

	const queryResult = useQuery(query, { ...options, ssr: !data });

	return useMemo(() => (data ? { ...queryResult, data, loading: false } : queryResult), [data, queryResult]);

}


@abroadwin abroadwin reopened this Nov 12, 2019
@jerelmiller
Copy link
Member

Hey @abroadwin 👋

I know this request is a bit old now, but with the introduction of React 18 and its new SSR APIs, we plan to adopt the new React SSR capabilities which leverage Suspense. We are unlikely to do much work on the existing SSR solution given the new paradigm moving forward.

I'd recommend following our Suspense RFC for updates. We'll have some updates in regards to SSR pretty soon. Thanks!

@jerelmiller jerelmiller closed this as not planned Won't fix, can't repro, duplicate, stale Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants