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

Inside StrictMode useSubscription subscribes twice #6037

Closed
anajavi opened this issue Mar 9, 2020 · 45 comments
Closed

Inside StrictMode useSubscription subscribes twice #6037

anajavi opened this issue Mar 9, 2020 · 45 comments

Comments

@anajavi
Copy link

anajavi commented Mar 9, 2020

Intended outcome:

useSubscription should subscribe only once when run inside <StrictMode>

Actual outcome:

useSubscription subscribes twice to the same subscription. I think that this happens because <StrictMode> intentionally runs component bodies twice. I suppose that same thing could happen in concurrent mode too.

This results in double network traffic. Also, the second subscription won't get unsubscribed when the useSubscription is unmounted.

This can be inspected on chrome network tab:
usesubscription_chrome
in the picture, id 15 and 16 are the single useSubscription and receive the same data. After unmount the id 16 was closed and id 15 was left receiving data(not pictured).

I am not familiar with apollo-client codebase, but I think that this is result of the following line being run in function body instead of inside useEffect:

return subscriptionData.execute(result);

How to reproduce the issue:

Any subscription should behave the same when run in StrictMode.

const SubComponent = ({vesselId}) => {
  const { data } = useSubscription(vesselStatus, {
    variables: { vesselId }
  });
  return null;
}
<StrictMode>
  <SubComponent vesselId="245" />
</StrictMode>

Workaround is to not use StrictMode, but I'm afraid that will result in code that breaks when concurrent mode lands in React.

Versions

System:
OS: macOS 10.15.3
Binaries:
Node: 12.16.1 - ~/.nvm/versions/node/v12.16.1/bin/node
Yarn: 1.22.0 - ~/.nvm/versions/node/v12.16.1/bin/yarn
npm: 6.13.4 - ~/.nvm/versions/node/v12.16.1/bin/npm
Browsers:
Chrome: 80.0.3987.132
Firefox: 73.0.1
Safari: 13.0.5
npmPackages:
@apollo/client: ^3.0.0-beta.39 => 3.0.0-beta.39
@apollo/link-ws: ^2.0.0-beta.3 => 2.0.0-beta.3
apollo-server-express: ^2.11.0 => 2.11.0
npmGlobalPackages:
apollo-codegen: 0.20.2
apollo: 2.24.0

@anajavi
Copy link
Author

anajavi commented Mar 11, 2020

If there is interest in fixing this, I can try to make a PR of a failing test case? Not sure it can be done cleanly though.

@flootr
Copy link

flootr commented Apr 12, 2020

I'm having the same issue. <StrictMode /> e.g. seems to be enabled for create-react-app by default and causes the useSubscription hook to subscribe twice on initial render.

Update: strict mode also causes trouble when setting a pollInterval in useQuery which then keeps polling even if the component is unmounted.

@anajavi
Copy link
Author

anajavi commented Apr 14, 2020

Update: strict mode also causes trouble when setting a pollInterval in useQuery which then keeps polling even if the component is unmounted.

Both of these hooks will then fail under concurrent mode.

@Tadimsky
Copy link

Yep, running into this issue as well - all of my useSubscription's are subscribing twice.

@estubmo
Copy link

estubmo commented May 6, 2020

Experiencing the same issue. Any news?

@esseswann
Copy link

Confirmed

@csteuer
Copy link

csteuer commented May 29, 2020

We have the same problem. Any updates?

@brunoreis
Copy link

brunoreis commented Jun 14, 2020

I have just started playing with Subscriptions. So I have a fresh CRA app here, and it is duplicating subscriptions. When I remove StrictMode, it stops.

@boucherv-project
Copy link

Same issue for me

@MaxTibs
Copy link

MaxTibs commented Jul 7, 2020

Notice that this is true only if you're in development mode.
Try to run a production build and this effect will go away.

@Fxlr8
Copy link

Fxlr8 commented Jul 8, 2020

Same thing for me. Wasted a couple of hours. I thought that I was going crazy.

@stephencorwin
Copy link

Try to run a production build and this effect will go away.

That might hide the symptoms, but having to run a production build when doing active development would get tedious and annoying real fast. We need a real solution for this issue.

@MaxTibs
Copy link

MaxTibs commented Jul 16, 2020

I agree, this is annoying. In my case I listen for events and display an alert for each of them in the UI which turns out to be duplicated. However the problem does not come from Apollo-Client, it is React strict mode behavior.. I don't think we can expect a fix coming from Apollo to remove this behavior since it is external to Apollo-Client.

I can see multiple way to limit the side effect of this behavior :

  • Remove the React.StrictMode (not ideal if you want React to check your code)
  • You could write tests for your components to make sure they behave correctly and then turn them off (or not) with global variables on a development environment to avoid the duplicated subscription behavior.
  • If you store data in a cache (e.g. Redux store) or display a table based on the event you receive from a subscription, use a dictionary instead of an array to easily override the duplicated data.

@anajavi
Copy link
Author

anajavi commented Jul 16, 2020

I agree, this is annoying. In my case I listen for events and display an alert for each of them in the UI which turns out to be duplicated. However the problem does not come from Apollo-Client, it is React strict mode behavior.. I don't think we can expect a fix coming from Apollo to remove this behavior since it is external to Apollo-Client.

There is a reason for strict mode rendering twice. It is to prevent bugs in the coming concurrent mode. So this should be fixed in apollo-client.

@igor10k
Copy link

igor10k commented Aug 28, 2020

The worst thing is that those duplicated observables leak and never get destroyed, so when you do client.resetStore() all of them get refetched and this most probably will throw errors because the auth token is not set anymore.
I'm surprised that this issue is completely ignored for almost half a year already.

@semoal
Copy link

semoal commented Sep 7, 2020

Screenshot 2020-09-07 at 15 14 50
I'm still getting this issue, returning twice when is triggered the subscription

@davegariepy
Copy link

experiencing this now, seeing duplicate subscriptions. First thing I thought of was to use useEffect, then I found this thread.

@raymclee
Copy link

im having this issue also

@neildotvn
Copy link

I have the same issue

@ybatsiun
Copy link

ybatsiun commented Nov 2, 2020

The same issue

@MR-Neto
Copy link

MR-Neto commented Nov 4, 2020

Same issue

@khaphannm
Copy link

same issue here, please help

@kelvin2200
Copy link

https://github.com/apollographql/subscriptions-transport-ws/blob/master/src/client.ts

can't wrap my head around why this is happening
but in DEV mode

this.applyMiddlewares (which returns a promise)

seems to be resolving twice for the same subscription
BUT... with a different id

private executeOperation(options: OperationOptions, handler: (error: Error[], result?: any) => void): string {
    if (this.client === null) {
      this.connect();
    }

    const opId = this.generateOperationId();
    this.operations[opId] = { options: options, handler };

    this.applyMiddlewares(options)
      .then(processedOptions => {
        this.checkOperationOptions(processedOptions, handler);
        if (this.operations[opId]) {
          this.operations[opId] = { options: processedOptions, handler };
          this.sendMessage(opId, MessageTypes.GQL_START, processedOptions);
        }
      })
      .catch(error => {
        this.unsubscribe(opId);
        handler(this.formatErrors(error));
      });

    return opId;
}

the odd thing is, even though this condition exists

if (this.operations[opId]) {
      this.operations[opId] = { options: processedOptions, handler };
      this.sendMessage(opId, MessageTypes.GQL_START, processedOptions);
}

AND opId is created only once BEFORE calling

this.applyMiddlewares(...)

The second time the promise is resolved
...the same condition evaluates to true
for an id that is not created and shouldn't exist in this.operations

didn't dig too much through the code, so this... could be totally unrelated to why the subscription is fired twice
but maybe its a good starting point

@Dezzymei
Copy link

Dezzymei commented Dec 2, 2020

👍 same issue

@matbour
Copy link

matbour commented Dec 16, 2020

I also can confirm the issue. Does someone have an idea of why this is happening? I don't really understand the relationship between <React.StrictMode/> and the subscriptions...

@anajavi
Copy link
Author

anajavi commented Dec 16, 2020

I also can confirm the issue. Does someone have an idea of why this is happening? I don't really understand the relationship between <React.StrictMode/> and the subscriptions...

Because in development StrictMode causes function component bodies to double-execute: https://reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects

The useSubscription hook is breaking contract with react by starting the subscription in function body.

@matbour
Copy link

matbour commented Dec 19, 2020

I also can confirm the issue. Does someone have an idea of why this is happening? I don't really understand the relationship between <React.StrictMode/> and the subscriptions...

Because in development StrictMode causes function component bodies to double-execute: https://reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects

The useSubscription hook is breaking contract with react by starting the subscription in function body.

Thank you very much for the precision. I'll try to look on the useSubscription hook and maybe submit a PR to fix this.

@sgentile
Copy link

I get multiple as well

@tehpsalmist
Copy link

At least I now know this is only a development issue, but sheesh, what a waste of time.

@highri5e
Copy link

highri5e commented Mar 9, 2022

still an issue

@hwillson hwillson added the 🔍 investigate Investigate further label May 31, 2022
@AlexanderWells-diamond
Copy link

Also seeing the same issue.

@bfeucht-wof
Copy link

bfeucht-wof commented Aug 1, 2022

I am seeing this issue in both development and production mode, without strict mode. I'm not sure if this is relevant, but the call stack is different in

In production mode, I get

Error: Observable cancelled prematurely
    at t.removeObserver (LocalState.ts:147:27)
    at n (LocalState.ts:62:25)
    at b (TextareaAutosize.js:88:7)
    at e.unsubscribe (TextareaAutosize.js:203:7)
    at b (TextareaAutosize.js:93:21)
    at e.unsubscribe (TextareaAutosize.js:203:7)
    at resolveProps.js:118:20
    at Lu (scheduler.production.min.js:262:224)
    at t.unstable_runWithPriority (index.js:18:343)
    at Ko (scheduler.production.min.js:122:325)

In dev mode I get:

Error: Observable cancelled prematurely
    at Concast.removeObserver (Concast.ts:147:1)
    at Concast.ts:62:1
    at cleanupSubscription (module.js:88:1)
    at Subscription.unsubscribe (module.js:203:1)
    at cleanupSubscription (module.js:93:1)
    at Subscription.unsubscribe (module.js:203:1)
    at useSubscription.ts:118:1
    at HTMLUnknownElement.callCallback (react-dom.development.js:3945:1)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994:1)
    at invokeGuardedCallback (react-dom.development.js:4056:1)
    at flushPassiveEffectsImpl (react-dom.development.js:23543:1)
    at unstable_runWithPriority (scheduler.development.js:468:1)
    at runWithPriority$1 (react-dom.development.js:11276:1)
    at flushPassiveEffects (react-dom.development.js:23447:1)
    at react-dom.development.js:23324:1
    at workLoop (scheduler.development.js:417:1)
    at flushWork (scheduler.development.js:390:1)
    at MessagePort.performWorkUntilDeadline (scheduler.development.js:157:1)

@yovanoc
Copy link

yovanoc commented Nov 5, 2022

same issue, but like others, only in dev

amosjyng added a commit to amosjyng/ice that referenced this issue Jan 19, 2023
On dev (but not on prod), TracePage is unmounted and remounted, but isMounted.current stays false and the trace tree never renders. This appears to be due to React.StrictMode, as seen in this other bug on another project: apollographql/apollo-client#6037
@alessbell alessbell self-assigned this Apr 3, 2023
@alessbell alessbell removed their assignment Apr 25, 2023
@andershhh
Copy link

andershhh commented Jan 31, 2024

@alessbell @anajavi do you know if this problem was ever resolved in some way? I'm experiencing this exact issue using "@apollo/client": "^3.8.10". I'd prefer to keep strict mode on for my project.

@alessbell
Copy link
Member

Hi @andershhh 👋 There isn't a simple fix here since we'll have to change the mechanism by which we register and track subscriptions, so yes this is still an issue. The workaround for now is to disable strict mode.

@andershoegh
Copy link

Hi @andershhh 👋 There isn't a simple fix here since we'll have to change the mechanism by which we register and track subscriptions, so yes this is still an issue. The workaround for now is to disable strict mode.

Hi! Thank you for the clarification and recommendation on how to resolve the issue in the meantime. We've opted, currently, to solve it in the few places where it causes an issue, in the code instead of turning off strict mode. This is okay for now, while the use of the subscribetomore feature is relatively small for us.

Thank you for all the work all the contributors are putting into the project 😊🙏🏻

@UchihaYuki
Copy link

Any updates? It's been 4 years now.

@phryneas
Copy link
Member

phryneas commented Jul 1, 2024

@UchihaYuki #11863 changes the subscription to useSyncExternalStore, which should fix the problem. It will be released in 3.11.

@phryneas
Copy link
Member

phryneas commented Aug 1, 2024

Version 3.11 has been released with a rewrite of useSubscription.

If you find any problems with useSubscription after 3.11, please open a new issue. I'm closing this issue since the code that originally caused this issue is not part of the library anymore.

@phryneas phryneas closed this as completed Aug 1, 2024
Copy link
Contributor

github-actions bot commented Aug 1, 2024

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

Copy link
Contributor

github-actions bot commented Sep 1, 2024

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 1, 2024
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