-
Notifications
You must be signed in to change notification settings - Fork 92
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
add TypeScript type definitions. #54
base: master
Are you sure you want to change the base?
Conversation
I'm kinda in the middle on this PR. Our main service is written in TypeScript so I see the advantage of having this available (e.g. avoiding At the same time, I wonder if this is a community standard, where packages are supporting TypeScript definitions --especially when it's not used in src. One example I found: https://github.com/axios/axios What do you think? @ronkorving Happy to test as well. |
I like this. However... What are the best practices for maintaining this? How can people who don't use TypeScript still safely make contributions without breaking the type definitions file all the time? |
@ronkorving I can add a test case for the type definitions, which is a simple check if end-user calls match the described API, and compiling it without generating an output ( // test.ts
import * as iap from "../";
iap.verifyPayment("google", {
receipt: {},
productId: "product",
packageName: "com.example",
secret: "123",
}, (err, response) => {
}); I just noticed that this library doesn't have unit testing for JavaScript, which can also be difficult for safely making contributions 😱 |
Yeah some tests would go a long way for this package. Probably another issue or pull request needed for that effort. I'm curious if we would just adopt some unit tests with mocked data or setup a test sandbox that @ronkorving would allow us to run against. in-app-purchase package does this but I think its for the sake of end-to-end testing. One downside to this is the burden falls on the owner to maintain --I'm not too crazy about this option. I'm in favor of mock data with unit test. |
I couldn't agree more. I would love a test suite very much. The nature of this packages seems to make it so that it's hard to have a dedicated integration test, so mocking may just have to do the trick. I'm all for it 👍 |
Hey there,
I've added TypeScript type definitions for the
verifyPayment
method, which includes:Platform
type with supported platformsPayment
interfaceResponse
interfaceCheers!