TypeScript Improvements

[LinusU]

Recently started using the library in a TypeScript project and noticed a few things that could be improved. Thanks for a great library!

Here are some examples of the improvements seen from VS Code:

I haven't had time to type up the response for validateReceiptAndroid yet, but hopefully there will be another pull request soon™ 🚀

[LinusU]

I'm removing this because it wasn't actually exported from index.js, so trying to use it would just result in undefined at runtime.

I'm planning on fixing this by normalizing the type property so that it's the same on Android and iOS, expect a PR for that soon™ 😎

[cometkim]

That's good plan. But breaks compatibility, so I think we'll have to deal with this change separately.

[LinusU]

But breaks compatibility

The change in this PR wouldn't though? Because there are no SkuTypes actually exported from index.js?

I still think that we should change it though, shouldn't be a problem to introduce breaking changes since we are in an alpha release?

[cometkim]

Ah, sorry. I confused. Removing SkuTypes variable is good.

But SkuType type

export interface Product {
-  type: SkuTypeAndroid | SkuTypeIOS;
+  type: 'inapp' | 'iap'

export function getProducts(skus: string[]): Promise<Product<string>[]>;

It looks typeof getProducts is not valid for Android until the SkuType is nomalized.

[LinusU]

It looks typeof getProducts is not valid for Android until the SkuType is nomalized.

I believe it is, 'inapp' is for Android and 'iap' for iOS.

Note that I split up Product and Subscription since getProducts only returns products (I think at some point in history it also included subscriptions on iOS 🤔)

[cometkim]

I'm not sure about. Alright, If it's case insensitive. 👌

[LinusU]

I don't think it's case insensitive, but index.js is using the lower case version so I assumed that that was the correct one 👍

[LinusU]

I removed RNVersion from here as well, my reasoning is that passing in undefined will actually work since undefined < 54 evaluates to false. Anyhow, when we merge #238 that parameter will be gone forever 🎉

[cometkim]

Looks neat! great job @LinusU.

Made a few change requests and everything else LGTM.

[cometkim]

Missing is-retryable property.

is-retryable: Retry validation for this receipt. Only applicable to status codes 21100-21199 (listed in Table 2-1)

[LinusU]

Will make PR for this 👍

[cometkim]

That's good plan. But breaks compatibility, so I think we'll have to deal with this change separately.

[hyochan]

@cometkim @LinusU Thank for your work!

[cometkim]

Hey @dooboolab and @LinusU, I was noticed about "Mapped types on tuples" support in TypeScript 3.1

It looks like we can make this declaration be exact and short, but should drop support below versions of TS.

export function getProducts<A extends string, B extends string, C extends string, D extends string, E extends string, F extends string>(skus: [A, B, C, D, E, F]): Promise<[Product<A>, Product<B>, Product<C>, Product<D>, Product<E>, Product<F>]>;
export function getProducts<A extends string, B extends string, C extends string, D extends string, E extends string>(skus: [A, B, C, D, E]): Promise<[Product<A>, Product<B>, Product<C>, Product<D>, Product<E>]>;
export function getProducts<A extends string, B extends string, C extends string, D extends string>(skus: [A, B, C, D]): Promise<[Product<A>, Product<B>, Product<C>, Product<D>]>;
export function getProducts<A extends string, B extends string, C extends string>(skus: [A, B, C]): Promise<[Product<A>, Product<B>, Product<C>]>;
export function getProducts<A extends string, B extends string>(skus: [A, B]): Promise<[Product<A>, Product<B>]>;
export function getProducts<A extends string>(skus: [A]): Promise<[Product<A>]>;
export function getProducts(skus: string[]): Promise<Product<string>[]>;

How do you think about?

[LinusU]

[...] but should drop support below versions of TS.

works for me 👍

[hyochan]

@cometkim @LinusU Actually, I don't know how to manage this one since it isn't downward compatible. Would this be ok to just drop the lower version support?

[cometkim]

@dooboolab It's not that important while it is not a dependency or peer dependency.

[cometkim]

@LinusU I just tried to use tuple mapping, but keyof does not support explicit inference of arrays or tuple entries.. it looks always string | number | symbol and the number is incompatible with Product<ID extends string>.

Closed this topic..