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

Typescript issues #224

Closed
bobbyjcolley opened this issue Oct 21, 2021 · 4 comments · Fixed by #225
Closed

Typescript issues #224

bobbyjcolley opened this issue Oct 21, 2021 · 4 comments · Fixed by #225

Comments

@bobbyjcolley
Copy link
Contributor

Hi RNAMA!

I've noticed an issue where the typescript typings for the NativeMediaView component callbacks are slightly off.

https://github.com/ammarahm-ed/react-native-admob-native-ads/blob/master/index.d.ts#L434-L438

I didn't see anything which leads me to think that these should be arrow functions which do not return a value. This would be the fix:

onVideoProgress?: (progress: {
      duration: string;
      currentTime: string;
    }) => void;
onVideoMute?: (muted: boolean) => void;
@bobbyjcolley
Copy link
Contributor Author

bobbyjcolley commented Oct 21, 2021

@ammarahm-ed I've noticed a bunch of typescript issues while setting up the example with my own code. I'd be happy to spend some time going through the example and either converting the repo to typescript and using a build tool like react native builder bob or another.

Some other issues include the documentation saying targetingOptions and mediation options are not required, but typescript indicates they are required.

Let me know your thoughts!

@ammarahm-ed
Copy link
Owner

Hey thanks for pointing out and sending a PR. There are some ts issues, I think they can be resolved. Rewriting in typescript isn't an option at the moment.

You can set both to not required in index.d.ts file as they are not needed.

@bobbyjcolley
Copy link
Contributor Author

Thanks. I did update the types to reflect that in my PR (#225)!

@ammarahm-ed
Copy link
Owner

I have also fixed rest of the issues.

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 a pull request may close this issue.

2 participants