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

Associate Apollo context with React.createContext (instead of using a local WeakMap) again #8798

Merged
merged 4 commits into from
Sep 17, 2021

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Sep 16, 2021

Should fix #8790, by allowing multiple copies of @apollo/client (uncommon) to share the same React.createContext[contextKey] object.

A lot has changed since #7371, but I am not worried about React.createContext being undefined, because (in this module, ApolloContext.ts), the React object is now a namespace object (thanks to import * as React from 'react'), and we call React.createContext unconditionally within getApolloContext already, so React.createContext must be defined when getApolloContext is called, or this code would be totally broken.

This implementation uses React.createContext[contextKey] instead of React[contextKey] (as in #7371) because the React namespace object is likely frozen/non-extensible, whereas React.createContext is an ordinary function object.

@benjamn benjamn added this to the v3.4.x patch releases milestone Sep 16, 2021
@benjamn benjamn self-assigned this Sep 16, 2021
@benjamn benjamn force-pushed the associate-context-with-React.createContext-again branch from 26810b3 to b9c3f0a Compare September 16, 2021 20:46
benjamn added a commit that referenced this pull request Sep 16, 2021
@benjamn benjamn force-pushed the associate-context-with-React.createContext-again branch from b9c3f0a to 11f485e Compare September 16, 2021 21:19
Should fix #8790, by allowing multiple copies of `@apollo/client`
(uncommon) to share the same `React.createContext[contextKey]` object.
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

Looks great @benjamn!

@benjamn benjamn merged commit 6aabfc7 into main Sep 17, 2021
@benjamn benjamn deleted the associate-context-with-React.createContext-again branch September 17, 2021 15:27
@ruben-nogueira
Copy link

ruben-nogueira commented Sep 20, 2021

@benjamn @hwillson My vue 2.6 projects are failing to run/build after upgrading the apollo client dependency to 3.4.12.

Log:

Step #2: > xxxxxxxxxxxx@0.1.0 build /workspace
Step #2: > vue-cli-service build
Step #2: 
Step #2:  ERROR  Error loading /workspace/vue.config.js:
Step #2:  ERROR  Error: Cannot find module 'react'
Step #2: Require stack:
Step #2: - /workspace/node_modules/@apollo/client/react/context/context.cjs.js
Step #2: - /workspace/node_modules/@apollo/client/react/react.cjs.js
Step #2: - /workspace/node_modules/@apollo/client/main.cjs.js
Step #2: - /workspace/node_modules/@apollo/client/utilities/utilities.cjs.js
Step #2: - /workspace/node_modules/@apollo/client/link/core/core.cjs.js
Step #2: - /workspace/node_modules/@apollo/client/core/core.cjs.js
Step #2: - /workspace/xxxxxxxx.js
Step #2: - /workspace/yyyyy.js
Step #2: - /workspace/zzzzzzz.js
Step #2: - /workspace/vue.config.js
Step #2: - /workspace/node_modules/@vue/cli-shared-utils/lib/module.js
Step #2: - /workspace/node_modules/@vue/cli-shared-utils/index.js
Step #2: - /workspace/node_modules/@vue/cli-service/bin/vue-cli-service.js
Step #2: Error: Cannot find module 'react'
Step #2: Require stack:
Step #2: - /workspace/node_modules/@apollo/client/react/context/context.cjs.js
Step #2: - /workspace/node_modules/@apollo/client/react/react.cjs.js
Step #2: - /workspace/node_modules/@apollo/client/main.cjs.js
Step #2: - /workspace/node_modules/@apollo/client/utilities/utilities.cjs.js
Step #2: - /workspace/node_modules/@apollo/client/link/core/core.cjs.js
Step #2: - /workspace/node_modules/@apollo/client/core/core.cjs.js
Step #2: - /workspace/xxxxx.js
Step #2: - /workspace/yyyyy.js
Step #2: - /workspace/zzzzzzzzz.js
Step #2: - /workspace/vue.config.js
Step #2: - /workspace/node_modules/@vue/cli-shared-utils/lib/module.js
Step #2: - /workspace/node_modules/@vue/cli-shared-utils/index.js
Step #2: - /workspace/node_modules/@vue/cli-service/bin/vue-cli-service.js
Step #2:     at Function.Module._resolveFilename (internal/modules/cjs/loader.js:893:15)
Step #2:     at Function.Module._load (internal/modules/cjs/loader.js:743:27)
Step #2:     at Module.require (internal/modules/cjs/loader.js:965:19)
Step #2:     at require (internal/modules/cjs/helpers.js:88:18)
Step #2:     at Object.<anonymous> (/workspace/node_modules/@apollo/client/react/context/context.cjs.js:6:13)
Step #2:     at Module._compile (internal/modules/cjs/loader.js:1076:30)
Step #2:     at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
Step #2:     at Module.load (internal/modules/cjs/loader.js:941:32)
Step #2:     at Function.Module._load (internal/modules/cjs/loader.js:782:14)
Step #2:     at Module.require (internal/modules/cjs/loader.js:965:19)

Reverting to 3.4.11 fixes the problem. Thanks.

@@ -1,4 +1,5 @@
import { Observable } from "./Observable";
import { canUseSymbol } from "..";
Copy link
Member Author

Choose a reason for hiding this comment

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

This line seems to have caused a number of problems, hopefully all fixed by #8817.

@benjamn
Copy link
Member Author

benjamn commented Sep 20, 2021

@ruben-nogueira Please try @apollo/client@3.4.13 (which includes #8817) when you have a chance!

@ruben-nogueira
Copy link

@benjamn it's okay. Thanks again. 👍

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

Successfully merging this pull request may close these issues.

Multiple apollo-client instances will not reuse the same Apollo Context in React
3 participants