From 39f7bffdf695eab03b590c1cfe077081f56b10c1 Mon Sep 17 00:00:00 2001 From: kazekyo Date: Wed, 11 May 2022 22:00:29 +0900 Subject: [PATCH 1/5] Fix useSubscription bug in React v18 StrictMode (#9664) React18 automatically unmounts and remounts components in StrictMode. When React remount a component, the previous state is restored. However, if the previous state is reused, the subscription will not work. So we should recreate the necessary object when React remounts the component. --- src/react/hooks/useSubscription.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/react/hooks/useSubscription.ts b/src/react/hooks/useSubscription.ts index f1335048b53..ff68f43d496 100644 --- a/src/react/hooks/useSubscription.ts +++ b/src/react/hooks/useSubscription.ts @@ -9,7 +9,7 @@ import { SubscriptionHookOptions, SubscriptionResult } from '../types/types'; -import { OperationVariables } from '../../core'; +import { FetchResult, Observable, OperationVariables } from '../../core'; import { useApolloClient } from './useApolloClient'; export function useSubscription( @@ -25,18 +25,20 @@ export function useSubscription( variables: options?.variables, }); - const [observable, setObservable] = useState(() => { + const [observable, setObservable] = useState> | null>(null); + + useEffect(() => { if (options?.skip) { - return null; + return; } - return client.subscribe({ + setObservable(client.subscribe({ query: subscription, variables: options?.variables, fetchPolicy: options?.fetchPolicy, context: options?.context, - }); - }); + })); + }, []); const ref = useRef({ client, subscription, options }); useEffect(() => { From 6c3e28af405ac0417312a716959a625976da4ec7 Mon Sep 17 00:00:00 2001 From: kazekyo Date: Thu, 12 May 2022 00:03:35 +0900 Subject: [PATCH 2/5] Fix to affect only remount --- src/react/hooks/useSubscription.ts | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/react/hooks/useSubscription.ts b/src/react/hooks/useSubscription.ts index ff68f43d496..44a84eea053 100644 --- a/src/react/hooks/useSubscription.ts +++ b/src/react/hooks/useSubscription.ts @@ -9,7 +9,7 @@ import { SubscriptionHookOptions, SubscriptionResult } from '../types/types'; -import { FetchResult, Observable, OperationVariables } from '../../core'; +import { OperationVariables } from '../../core'; import { useApolloClient } from './useApolloClient'; export function useSubscription( @@ -25,20 +25,23 @@ export function useSubscription( variables: options?.variables, }); - const [observable, setObservable] = useState> | null>(null); - - useEffect(() => { + const [observable, setObservable] = useState(() => { if (options?.skip) { - return; + return null; } - setObservable(client.subscribe({ + return client.subscribe({ query: subscription, variables: options?.variables, fetchPolicy: options?.fetchPolicy, context: options?.context, - })); - }, []); + }); + }); + + const [canResetObservable, setCanResetObservable] = useState(false); + useEffect(() => { + return () => setCanResetObservable(true); + }, []) const ref = useRef({ client, subscription, options }); useEffect(() => { @@ -48,7 +51,7 @@ export function useSubscription( } if (options?.skip) { - if (!options?.skip !== !ref.current.options?.skip) { + if (!options?.skip !== !ref.current.options?.skip || canResetObservable) { setResult({ loading: false, data: void 0, @@ -56,6 +59,7 @@ export function useSubscription( variables: options?.variables, }); setObservable(null); + setCanResetObservable(false); } } else if ( shouldResubscribe !== false && ( @@ -64,7 +68,7 @@ export function useSubscription( options?.fetchPolicy !== ref.current.options?.fetchPolicy || !options?.skip !== !ref.current.options?.skip || !equal(options?.variables, ref.current.options?.variables) - ) + ) || canResetObservable ) { setResult({ loading: true, @@ -78,10 +82,11 @@ export function useSubscription( fetchPolicy: options?.fetchPolicy, context: options?.context, })); + setCanResetObservable(false); } Object.assign(ref.current, { client, subscription, options }); - }, [client, subscription, options]); + }, [client, subscription, options, canResetObservable]); useEffect(() => { if (!observable) { From ec2604b929832a2f1bac025cd923e88c20a1673f Mon Sep 17 00:00:00 2001 From: kazekyo Date: Fri, 13 May 2022 12:22:51 +0900 Subject: [PATCH 3/5] Fix to use useRef instead of useState to avoid forcing a re-render --- src/react/hooks/useSubscription.ts | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/react/hooks/useSubscription.ts b/src/react/hooks/useSubscription.ts index 44a84eea053..7207657c64e 100644 --- a/src/react/hooks/useSubscription.ts +++ b/src/react/hooks/useSubscription.ts @@ -38,10 +38,12 @@ export function useSubscription( }); }); - const [canResetObservable, setCanResetObservable] = useState(false); + const canResetObservableRef = useRef(false); useEffect(() => { - return () => setCanResetObservable(true); - }, []) + return () => { + canResetObservableRef.current = true; + }; + }, []); const ref = useRef({ client, subscription, options }); useEffect(() => { @@ -51,7 +53,7 @@ export function useSubscription( } if (options?.skip) { - if (!options?.skip !== !ref.current.options?.skip || canResetObservable) { + if (!options?.skip !== !ref.current.options?.skip || canResetObservableRef.current) { setResult({ loading: false, data: void 0, @@ -59,16 +61,16 @@ export function useSubscription( variables: options?.variables, }); setObservable(null); - setCanResetObservable(false); + canResetObservableRef.current = false; } } else if ( - shouldResubscribe !== false && ( - client !== ref.current.client || - subscription !== ref.current.subscription || - options?.fetchPolicy !== ref.current.options?.fetchPolicy || - !options?.skip !== !ref.current.options?.skip || - !equal(options?.variables, ref.current.options?.variables) - ) || canResetObservable + (shouldResubscribe !== false && + (client !== ref.current.client || + subscription !== ref.current.subscription || + options?.fetchPolicy !== ref.current.options?.fetchPolicy || + !options?.skip !== !ref.current.options?.skip || + !equal(options?.variables, ref.current.options?.variables))) || + canResetObservableRef.current ) { setResult({ loading: true, @@ -82,11 +84,11 @@ export function useSubscription( fetchPolicy: options?.fetchPolicy, context: options?.context, })); - setCanResetObservable(false); + canResetObservableRef.current = false; } Object.assign(ref.current, { client, subscription, options }); - }, [client, subscription, options, canResetObservable]); + }, [client, subscription, options, canResetObservableRef.current]); useEffect(() => { if (!observable) { From 2c452288952968dae487c0fc8ce4712486532fb9 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 16 May 2022 16:59:16 -0400 Subject: [PATCH 4/5] Bump bundlesize limit to 29.5kB (now 28.46kB). --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ff9b3fd119b..8fae2f6c817 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ { "name": "apollo-client", "path": "./dist/apollo-client.min.cjs", - "maxSize": "29.45kB" + "maxSize": "29.5kB" } ], "engines": { From e62f8f6d37dba2144d2a298b0a6d810ec561b77f Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 16 May 2022 17:02:31 -0400 Subject: [PATCH 5/5] Mention PR #9707 in CHANGELOG.md. --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f14898b674..7eab5e2a734 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ - Guarantee `Concast` cleanup without `Observable cancelled prematurely` rejection, potentially solving long-standing issues involving that error.
[@benjamn](https://github.com/benjamn) in [#9701](https://github.com/apollographql/apollo-client/pull/9701) +- Ensure `useSubscription` subscriptions are properly restarted after unmounting/remounting by React 18 in ``.
+ [@benjamn](https://github.com/benjamn) in [#9707](https://github.com/apollographql/apollo-client/pull/9707) + ### Improvements - Internalize `useSyncExternalStore` shim, for more control than `use-sync-external-store` provides, fixing some React Native issues.