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

ApolloProvider causing rerenders in consumers if ApolloProvider is rerendered even with the same client argument #7626

Closed
dsonck92 opened this issue Jan 28, 2021 · 5 comments

Comments

@dsonck92
Copy link

dsonck92 commented Jan 28, 2021

Intended outcome:
Whenever ApolloProvider is rerendered (e.g. if the parent rerenders) but receives the same client, this should not cause the context (used by useQuery, useMutation, but also the consumers) to force a rerender.

I tried to use ApolloProvider to provide the Apollo Client inside a separate React environment (React-Konva). I tried to utilize the new type policies to prevent passing down props by using queries that fetch data from the cache directly by id. All this again to optimize the rather slow React-Konva scene. However, even without the cache changing, simply moving the scene around (which rerenders the parent component of ApolloProvider and thus ApolloProvider itself) caused all objects in the scene with queries to rerender.

Actual outcome:
Instead of objects not being rerendered as long as no data changes in the Apollo cache, all objects rerender whenever something changes up regardless of whether client remains the same.

How to reproduce the issue:
Create an ApolloProvider in a component that will rerender, notice that all memoized components deeper down in the tree still rerender even when the stored cache value doesn't change. For a working example (with an alternative that doesn't have this issue) see:

https://codesandbox.io/s/apollo-context-rerenders-umlrx

From what I gather, the following happens:

  • ApolloProvider is a functional component
  • If this is used inside React-Konva, it will not find the ApolloContext.Provider value from above, which causes the embedded if to execute (value === undefined/null).
  • This embedded if uses Object.assign({}, context, { client }) to create a context.
  • However, given that it's a functional component, it will do this for every instantiation.
  • So each time, context is different and React will rerender all context dependent components, causing this issue.

Locally, I fixed this by simply using my own provider:

export const SimpleApolloProvider = ({client, children}) => {
	const ApolloContext = getApolloContext();

	// Use useMemo to prevent context from updating during
	// rerenders with the same client. Generally the client
	// will remain the same throughout the lifetime of the
	// application, but when {client} is used, (or as official
	// implementation context = Object.assign({}, context, { client }))
	// each rerender will create a new object, causing *all*
	// useQuery/useMutation hooks to rerender due to changed context
	// value.
	const context = useMemo(() => ({
		client
	}), [client])

	return (
		<ApolloContext.Provider value={context}>
			{children}
		</ApolloContext.Provider>
	)
}

I don't need the reuse feature from ApolloProvider here so this is sufficient for my use case. But I believe it's useful to cache the context object value so the (potentially many) consumers don't needlessly rerender.

Versions

  System:
    OS: Linux 5.10 Arch Linux
  Binaries:
    Node: 6.14.4 - ~/.nvm/versions/node/v6.14.4/bin/node
    Yarn: 1.21.1 - ~/.nvm/versions/node/v6.14.4/bin/yarn
    npm: 3.10.10 - ~/.nvm/versions/node/v6.14.4/bin/npm
  Browsers:
    Firefox: 84.0.2
  npmPackages:
    @apollo/client: ^3.3.7 => 3.3.7 
    apollo-link-queue: ^3.0.0 => 3.0.0 
    apollo-link-serialize: ^3.1.1 => 3.1.1 
    apollo-utilities: ^1.3.3 => 1.3.3 
@hammad0316
Copy link

Running into this issue as well on latest npm apollo 3.4.16

@tathagat2000
Copy link

Facing same issue.
Can you please check this @hwillson

@phryneas
Copy link
Member

@tathagat2000 thanks for the ping - a fix for this will go out in 3.8 - see #10869

@tathagat2000
Copy link

Thank you @phryneas

phryneas added a commit that referenced this issue May 16, 2023
* add failing tests

* ApolloProvider: ensure context value stability

fixes #7626

* adjust bundlesize

* ApolloContext: handling in React Server Components

* feedback from code review
@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2023

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants