search

uerceg / play-install-referrer-react-native

Play Install Referrer Library ported to React Native
MIT License
25 stars 2 forks source link

TypeScript definitions and ignore on non-Android platforms #10

Closed apfritts closed 2 years ago

apfritts commented 3 years ago

Hi team!

Thank you for this awesome library! I added some additional TypeScript definitions since this wasn't totally complete. Let me know if anything looks weird or if you'd like anything changed.

I'm also trying to get this to work as a no-op on iOS. I think I was successful. Let me know if anyone runs into problems.

I tried updating references to the version number, but I'm not sure if that interferes with your release process or not.

💃🏻

FRizzonelli commented 3 years ago

@uerceg Can we have this merged?

uerceg commented 3 years ago

Hey @FRizzonelli

Thanks for the PR. I am mostly AFK for next two weeks so unfortunately won't have a chance to take a deeper look. But will do as soon as I'm back. Will keep you posted.

FRizzonelli commented 3 years ago

Hey @FRizzonelli

Thanks for the PR. I am mostly AFK for next two weeks so unfortunately won't have a chance to take a deeper look. But will do as soon as I'm back. Will keep you posted.

Awesome :) Enjoy your next two weeks :)

giorgiofellipe commented 2 years ago

@uerceg any news on this?

naishe commented 2 years ago

@uerceg -- the code in the PR seems like a nice improvement. Could you please get this merged? It does two things:

  1. It adds type definition for the callback. It required me to look into Java code to see what type the timestamps are. I was thinking they'd be number. So, that would have been resolved much easily, if the typedef was merged
  2. It handles the case where the code is deployed on iOS without needing the developer to run a OS check.

@apfritts -- thanks for the PR. I am looking forward to get this merged.

uerceg commented 2 years ago

Hey guys,

Sorry for delay on my end, I wasn't managing to keep up with private projects lately. Will try to use this holidays season to check this. Will keep you posted and sorry one more time.

Cheers! 🍺🎄

apfritts commented 2 years ago

Happy new year @uerceg!

I've updated this PR with your other commits and prepped for a 1.1.8 release. Please let me know if anything looks off.

uerceg commented 2 years ago

Happy New Year to you too, @apfritts.

And thank you for the PR update! I started to check it and also in parallel to add another example app written in TypeScript where these changes might be tested. But I'm on vacation until January 10th, so will most definitely finish review and hopefully new release out of it after I'm back to coding life.

GiorgioBertolotti commented 2 years ago

@uerceg Any news with this PR? :)

uerceg commented 2 years ago

Hey guys, sorry for delay (quite embarrassing, sorry one more time). Yes, I'm currently working on it. Moved all the commits from this PR to https://github.com/uerceg/play-install-referrer-react-native/tree/v118 and checking stuff currently in there. And like written a bit above, checking also to add TypeScript example app to see if I can easily use it to test changes. 🔜 hopefully. 🤞

uerceg commented 2 years ago

@apfritts @GiorgioBertolotti 👋

Pardon my TypeScript ignorance, but would you be able to send me code snippet which shows how would you:

  1. Import plugin inside of the TypeScript app? (I assume this remains unchanged from how it's being done in JavaScript app, but just to double check)
  2. Make a call to the method to get install referrer information having new TypeScript typings from this PR in mind?

Many thanks in advance. 🍺

GiorgioBertolotti commented 2 years ago

Hi @uerceg, in both cases nothing changes from the example already provided in the README.md. At most you could change the call to getInstallReferrerInfo to be more explicit in typing, like this:

...
PlayInstallReferrer.getInstallReferrerInfo((installReferrerInfo: PlayInstallReferrerInfo | null, error: PlayInstallReferrerError | null) => {
...

But this is absolutely optional, since Typescript can figure out the type of installReferrerInfo and error by itself.

uerceg commented 2 years ago

@GiorgioBertolotti Thank you! This is exactly what I needed. I figured out this last part of your comment yesterday because even though I made TypeScript app, copy / pasting code from JavaScript version just automagically works. But I just wanted to check how would one attempt to use callback with types from TypeScript just to be sure that everything from index.d.ts works well. I'll give this a shot and hopefully have v1.1.8 ready to be released.

uerceg commented 2 years ago

Replaced with https://github.com/uerceg/play-install-referrer-react-native/pull/20. 🥁 🍺

uerceg commented 2 years ago

And v1.1.8 is live. Thank you guys for all your contribution (@apfritts 🍺) and patience. Give it a shot and in case you encounter any issues, feel free to open an issue. Cheers!