AppsFlyerSDK / appsflyer-react-native-plugin

AppsFlyer plugin for React Native
MIT License
273 stars 196 forks source link

setConsentData type issue #528

Closed artyorsh closed 3 months ago

artyorsh commented 4 months ago

Hi! ๐Ÿ‘‹

Firstly, thanks for your work on this project! ๐Ÿ™‚

Today I used patch-package to patch react-native-appsflyer@6.13.0 for the project I'm working on.

Here is the diff that solved my problem:

diff --git a/node_modules/react-native-appsflyer/index.d.ts b/node_modules/react-native-appsflyer/index.d.ts
index e1ae493..ba6ace1 100644
--- a/node_modules/react-native-appsflyer/index.d.ts
+++ b/node_modules/react-native-appsflyer/index.d.ts
@@ -115,8 +115,8 @@ declare module "react-native-appsflyer" {
     }

     export const AppsFlyerConsent: {
-        forGDPRUser: (hasConsentForDataUsage: boolean, hasConsentForAdsPersonalization: boolean) => void;
-        forNonGDPRUser: () => void;
+        forGDPRUser: (hasConsentForDataUsage: boolean, hasConsentForAdsPersonalization: boolean) => AppsFlyerConsentType;
+        forNonGDPRUser: () => AppsFlyerConsentType;
     }

     export type AppsFlyerConsentType = typeof AppsFlyerConsent;

This issue body was partially generated by patch-package.

chriskurzeja commented 4 months ago

I believe the proposed patch above doesn't go far enough in correcting the broken types. The following should actually fix the issue (ensuring that the object returned from forGDPRUser and forNonGDPRUser actually matches the expected type (an object containing isUserSubjectToGDPR, hasConsentForDataUsage and hasConsentForAdsPersonalization) based on the two platform specific implementations :

walterholohan commented 4 months ago

@amit-kremer93 can we get a fix for this merged. Seems like its very important with the March 6th deadline quickly approaching

buuuudzik commented 4 months ago

+1

walterholohan commented 4 months ago

@chriskurzeja @artyorsh thanks for the above. Super helpful.

Just wondering what method did you use to determine if the user was in a GDPR region or not? I was thinking to use the Timezone string to determine. Been keen to see if there is a better way

artyorsh commented 4 months ago

@chriskurzeja @artyorsh thanks for the above. Super helpful.

Just wondering what method did you use to determine if the user was in a GDPR region or not? I was thinking to use the Timezone string to determine. Been keen to see if there is a better way

A bit different for us since I work on "EU only" app. But I guess I'd rely on IP ranges.

amit-kremer93 commented 3 months ago

Fixed in 6.13.1