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

Wrong typescript typings for PurchasesIntroPrice #331

Closed
5 tasks done
xseignard opened this issue Feb 14, 2022 · 11 comments
Closed
5 tasks done

Wrong typescript typings for PurchasesIntroPrice #331

xseignard opened this issue Feb 14, 2022 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@xseignard
Copy link

xseignard commented Feb 14, 2022

Describe the bug
A clear and concise description of what the bug is. The more detail you can provide the faster our team will be able to triage and resolve the issue. Do not remove any of the steps from the template below. If a step is not applicable to your issue, please leave that step empty.

  1. Environment
    1. Platform: Android
    2. SDK version: 4.5.2
    3. OS version: 12
    4. Xcode/Android Studio version: not relevant
    5. React Native version: 0.67.2
    6. SDK installation (CocoaPods + version or manual): not relevant
    7. How widespread is the issue. Percentage of devices affected.
  2. Debug logs that reproduce the issue
  3. Steps to reproduce, with a description of expected vs. actual behavior
  • purchase a subscription with an intro price and note that PurchasesIntroPrice properties are not null
  • after a few time cancel it
  • purchase again and note that PurchasesIntroPrice properties are null
  1. Other information (e.g. stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow, etc.)

The above situation is expected, it is only the PurchasesIntroPrice that should be fixed with something like:

export interface PurchasesIntroPrice {
    /**
     * Price in the local currency.
     */
    readonly price: number | null;
    /**
     * Formatted price, including its currency sign, such as €3.99.
     */
    readonly priceString: string | null;
    /**
     * Number of subscription billing periods for which the user will be given the discount, such as 3.
     */
    readonly cycles: number | null;
    /**
     * Billing period of the discount, specified in ISO 8601 format.
     */
    readonly period: string | null;
    /**
     * Unit for the billing period of the discount, can be DAY, WEEK, MONTH or YEAR.
     */
    readonly periodUnit: string | null;
    /**
     * Number of units for the billing period of the discount.
     */
    readonly periodNumberOfUnits: number | null;
}

don't you think?

Additional context
Add any other context about the problem here.

Here is a screenshot of what I have in my app.
Screenshot_20220214-092821

@xseignard xseignard added the bug Something isn't working label Feb 14, 2022
@beylmk
Copy link
Contributor

beylmk commented Feb 14, 2022

Hey @xseignard, thanks for bringing this to our attention! You're definitely right, these should be marked nullable if they can come back as null. Will update here once fixed!

@vegaro
Copy link
Contributor

vegaro commented Feb 14, 2022

I think ideally, introPrice itself would be nullable instead. I don't think we intend the properties to be nullable. I am not sure how that happened. I think the object should be:

intro_price_period_unit: null
intro_price_string: null
introPrice: null

The intro_price... are deprecated and will be removed soon.

@beylmk
Copy link
Contributor

beylmk commented Feb 14, 2022

Going to confirm behavior in a sample app. Perhaps we have a bug in our hybrid-native middle layer, purchases-hybrid-common...

@xseignard
Copy link
Author

xseignard commented Feb 16, 2022

Hi there, thanks for your reactivity 🤗
@vegaro I can see that introPrice is itself nullable, and I was expecting to receive null for it instead of an object full of null properties.

So as @beylmk says, there may be some bug (creating the introPrice object instead of make it null) instead of a TS typings bug.

@joshdholtz
Copy link
Contributor

joshdholtz commented Feb 22, 2022

@xseignard Hey! I've been working on trying to reproduce this and I'm not having any luck 😔 I'm trying with SDK 4.5.2, on Android 12. I'm either getting:

  • "introPrice": {"cycles": 2, "period": "P1W", "periodNumberOfUnits": 7, "periodUnit": "DAY", "price": 0.99, "priceString": "$0.99"}
  • introPrice: null

Out Loud Thinking

introPrice (or it's other related fields) should also never turn null even after subscribing and cancelling 🤔 . It should only ever be null if there isn't any defined intro price for you IAP in Google Play. (At least, this is what the behavior should be)

This pull request (RevenueCat/purchases-hybrid-common#106) should have also prevented the introPrice from being a map/object of keys with null values.

Idea

It almost seems like this might be some reference or memory issue where the values in the introPrice object are turning to null. I have no real reason to back that up though or know how that's possible 😓

Follow Up Questions

  1. Is this only happening where you subscribe and cancel in the same app session? If you kill/force close the app and restart the app after cancelling, does everything run as expected or do you still get this error?
  2. Which call are you getting "introPrice": {"cycles": null, "period": null, "periodNumberOfUnits": null, "periodUnit": null, "price": null, "priceString": null} from? Is it getOfferings or getProducts (or something else)?
  3. What is your subscription and intro setup? Is your intro free, single, or recurring? Just want to make sure we are testing the exact same way you are 🤞

Thanks for reading through my thoughts and questions! Hoping we can figure out what's going for you soon ☺️


Also, there is another report of introductory price some times returning null for this Flutter user in this community post. I don't know if these. are related or not but it's pretty curious suspicious 😬

@jamesisaac
Copy link

jamesisaac commented Mar 7, 2022

@joshdholtz We ran into this issue in the following way:

  • Create an In-App Subscription on Google Play with an Introductory Price (yearly subscription with first year at reduced price).
  • Confirm this comes through correctly via RC's Purchases.getOfferings() call.
  • Remove the Introductory Price.
  • RC now returns a payload which breaks its TypeScript contract: "introPrice": {"cycles": null, "period": null, "periodNumberOfUnits": null, "periodUnit": null, "price": null, "priceString": null}

For us this then resulted in a runtime error (null is not an object) impacting all Android users who visited our pricing page, because we were checking for existence on introPrice as the types show, not the individual fields.

Ideal would be if introPrice could return null, but if not I think the typedefs should be updated as a priority... quite unexpected to have a settings change in Google Play Console lead to a runtime crash from a condition which the types rule as impossible.

@vegaro
Copy link
Contributor

vegaro commented Mar 18, 2022

@jamesisaac I was looking into this, and tried to reproduce if following your instructions, and I wasn't able to get your result. In any case, looking at the code of purchases-hybrid-common version 2.0.1 (the one react-native-purchases 4.5.2 uses) there is no way this could be returned.

We fixed this exact same bug in RevenueCat/purchases-hybrid-common#106, which was released in purchases-hybrid-common 1.11.2. react-native-purchases < 4.5.2 would have the bug and see what you are seeing for cases where intro price is null.

Is is maybe possible you are actually not testing with 4.5.2 code? Do you mind sharing the output of running ./gradlew androidDependencies inside the android folder of the React Native project where you are able to reproduce it?

Thanks! And sorry we haven't found a solution yet.

@vegaro vegaro added the status: waiting-for-reply Issues that are waiting for a reply label Mar 18, 2022
@jamesisaac
Copy link

@vegaro That is likely it, as our project was on 4.5.1! Apologies, missed the point that a fix for this was meant to have just landed in 4.5.2. I think before replying here I did check the 4.5.2 release notes and the changelogs linked for the other packages, and saw nothing relevant, but now I see that purchases-hybrid-common had multiple releases since 4.5.1. Thanks for clearing that up.

@stale stale bot removed the status: waiting-for-reply Issues that are waiting for a reply label Mar 18, 2022
@vegaro
Copy link
Contributor

vegaro commented Mar 21, 2022

Amazing! Mystery solved 😄

I will close this issue then. Thanks for opening anyway!

@vegaro vegaro closed this as completed Mar 21, 2022
@jamesisaac
Copy link

Just to clarify, I have no affiliation with @xseignard who originally opened this issue and does claim to be on 4.5.2 -- might be that they have a different scenario where this still can occur even with the latest fixes.

@vegaro
Copy link
Contributor

vegaro commented Mar 22, 2022

Thanks for clarifying! I had understood that you were working in the same project. Let's see if @xseignard was having the same issue or a different one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants