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

Unsubscribe before server returns the subscription id #11

Closed
chengyin opened this issue Dec 6, 2017 · 5 comments
Closed

Unsubscribe before server returns the subscription id #11

chengyin opened this issue Dec 6, 2017 · 5 comments

Comments

@chengyin
Copy link

chengyin commented Dec 6, 2017

Let's say I have a component with the following lifecycle events:

componentDidMount() {
  this.unsubscribe = this.props.subscribe();
}

componentWillUnmount() {
  this.unsubscribe();
}

In certain cases this component may be mounted and unmounted quickly (dropdown menu on hover, or dialogs that get dismissed instantly). This may lead to the situation where subscriptionId hasn't been returned from the server before this.unsubscribe is called.

With Absinthe, this will generate a server side error.

I don't think there is any API enables the component itself to avoid such situation. Should absinthe-socket handle this?

@mgtitimoli
Copy link
Member

Hi @chengyin,

Just to confirm, are you using absinthe-socket or any of the connector packages?

@chengyin
Copy link
Author

chengyin commented Dec 6, 2017

Actually let me emphasize that I’m not 100% sure if the quick unmounting is causing subscription id missing. But that’s one way to reproduce the issue.

And we are using the absinthe Apollo link.

@mgtitimoli
Copy link
Member

mgtitimoli commented Dec 6, 2017

If you are using absinthe/socket-apollo-link, then this should be handled automatically (here).

Where did you see the error?

@chengyin
Copy link
Author

chengyin commented Dec 7, 2017

Sorry, this was an issue on our end. I updated to the newest version of absinthe link in our master branch but I was testing on a branch with older version.

With the new version I hasn't seen any issues yet.

Sorry about the false report!

@chengyin chengyin closed this as completed Dec 7, 2017
@mgtitimoli
Copy link
Member

Great @chengyin, np, cheers.

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