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

Retain productsRequest #48

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Retain productsRequest #48

wants to merge 2 commits into from

Conversation

tarkus
Copy link

@tarkus tarkus commented Oct 6, 2016

On tvOS, productsRequest got released at end of the loadProducts func, and the following callbacks won't be fired. Declare it as a class level variable fix the problem.

@chirag04
Copy link
Owner

chirag04 commented Oct 6, 2016

are you sure this is a problem only for loadProducts? my guess is it could happen for purchaseProduct. Not sure if this is the right fix.

@tarkus
Copy link
Author

tarkus commented Oct 7, 2016

It could happen for purchaseProduct on tvOS, but without this fix, loadProducts will silently fail with no response, so we don't have a chance to know what happen next.

PS: this fix is for tvOS specifically, iOS just don't need it.

@tarkus
Copy link
Author

tarkus commented Oct 7, 2016

initWithProductIdentifiers:[NSSet setWithArray:productIdentifiers]];
productsRequest.delegate = self;
_callbacks[RCTKeyForInstance(productsRequest)] = callback;
[productsRequest start];
Copy link
Owner

@chirag04 chirag04 Dec 7, 2016

Choose a reason for hiding this comment

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

this will break when we have multiple requests happening. can you make it a map and in the delegate remove from the map?

gtebbutt added a commit to gtebbutt/react-native-in-app-utils that referenced this pull request Nov 27, 2017
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

Successfully merging this pull request may close these issues.

None yet

3 participants