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

[Bug]: 5.0.0 types #1540

Closed
3 tasks done
rares-lupascu opened this issue Aug 20, 2023 · 14 comments
Closed
3 tasks done

[Bug]: 5.0.0 types #1540

rares-lupascu opened this issue Aug 20, 2023 · 14 comments

Comments

@rares-lupascu
Copy link

rares-lupascu commented Aug 20, 2023

What happened?

which ones are correct?

OneSignal.User.login(id);
OneSignal.User.logout();

or

OneSignal.login(id);
OneSignal.logout();

the migration guide says the first ones are correct ... typescript says the second

Steps to reproduce?

1. read the migration guide

What did you expect to happen?

accurate types def

React Native OneSignal SDK version

5.0.0

Which platform(s) are affected?

  • iOS
  • Android

Relevant log output

Property 'login' does not exist on type 'typeof User'.ts(2339)

Code of Conduct

  • I agree to follow this project's Code of Conduct
@rgomezp
Copy link
Contributor

rgomezp commented Aug 21, 2023

Howdy @rares-lupascu ,
The second one is correct. We will make sure to update the migration guide to correct the error.

Thanks for bringing this to our attention.

Enjoy!

@manoharkharvi43
Copy link

Still the issue exits, in react-native Android it is working fine, we are able to set external id's. But working with ios, app is getting initialized, but not able to set external id's.

for initializing app:

OneSignal.initialize(id);
OneSignal.Debug.setLogLevel(LogLevel.Verbose);

OneSignal.Notifications.requestPermission(true);

OneSignal.Notifications.addEventListener("click", (event) => {
console.log("OneSignal: notification clicked:", event);
});

and for setting external id:

OneSignal.login(EXTERNAL_ID);

@rares-lupascu
Copy link
Author

it is no a pb with the docs only ... typescript throws an error ... so it is possible that the types were not updated

@rares-lupascu
Copy link
Author

rares-lupascu commented Aug 24, 2023

in the docs

await OneSignal.Notifications.hasPermission()

returns a boolean value not a Promise ... so why should we await it?

further more hasPermission() return object on iOS (at least) when permissions have not been granted:

{"permission": false}

@Romick2005
Copy link

In migration guide:

image
but in reality there are different names getId -> getPushSubscriptionId, getToken -> getPushSubscriptionToken

image

@Romick2005
Copy link

Methods getPushSubscriptionId and getPushSubscriptionToken are Promises only on Native side, but not for JS calls.
image
image
image

@KodyKendall
Copy link

I'm having a similar issue when reading the migration guide and the mobile SDK guide. In the mobile SDK guide, under react-native it says: OneSignal.User.PushSubscription.optIn();
But when I attempt that with package.json of "react-native-onesignal": "^5.0.0". I get the following error: Possible Unhandled Promise Rejection (id: 0): TypeError: Cannot read properties of undefined (reading 'optIn').

Same with: const pushSubscriptionId = await OneSignal.User.PushSubscription.getPushSubscriptionId();. I get a similar error: possible Unhandled Promise Rejection (id: 0): TypeError: Cannot read properties of undefined (reading 'getPushSubscriptionId')

@nan-li
Copy link
Contributor

nan-li commented Sep 6, 2023

Hi all, thank you for pointing out these inconsistencies with the migration guide.

We are fixing the migration guide.
There is no need to await any of these calls:

OneSignal.Notifications.hasPermission()
OneSignal.User.pushSubscription.getPushSubscriptionId()
OneSignal.User.pushSubscription.getPushSubscriptionToken()
OneSignal.User.pushSubscription.getOptedIn()

Additionally, we will need to fix the permission boolean (it is an object and need to be fixed).

@manoharkharvi43 - Can you explain the issue you have with iOS when logging into an external_id? What is the error?

@KodyKendall - Change the casing of pushSubscription to OneSignal.User.pushSubscription.optIn(). This was one of the changes we made after the beta releases (in 5.0.0 release)

@manoharkharvi43
Copy link

manoharkharvi43 commented Sep 7, 2023 via email

@Romick2005
Copy link

image
As it turns out from server error response "identity.external_id" need to be string, not a number.

@nan-li
Copy link
Contributor

nan-li commented Sep 8, 2023

Hi @manoharkharvi43, can you turn on verbose logging by OneSignal.Debug.setLogLevel(LogLevel.Verbose) and share the logs when you call OneSignal.login? We need more information to know what is not working for you.

Hi @Romick2005 yes the identity object needs to be string values, thanks for catching this error in the type.

@arnaudambro
Copy link

arnaudambro commented Sep 15, 2023

I'm having a similar issue when reading the migration guide and the mobile SDK guide. In the mobile SDK guide, under react-native it says: OneSignal.User.PushSubscription.optIn(); But when I attempt that with package.json of "react-native-onesignal": "^5.0.0". I get the following error: Possible Unhandled Promise Rejection (id: 0): TypeError: Cannot read properties of undefined (reading 'optIn').

Same with: const pushSubscriptionId = await OneSignal.User.PushSubscription.getPushSubscriptionId();. I get a similar error: possible Unhandled Promise Rejection (id: 0): TypeError: Cannot read properties of undefined (reading 'getPushSubscriptionId')

@ KodyKendall

it's not OneSignal.User.PushSubscription.optIn but OneSignal.User.pushSubscription.optIn

with p and not P

@FazilMuhammed
Copy link

FazilMuhammed commented Oct 11, 2023

@arnaudambro @rgomezp Anyone please help me how get player id / External ID react native oneSignal version 5+

const state = await OneSignal.getDeviceState();
const userID = state.userId;

this is not working

@nan-li
Copy link
Contributor

nan-li commented Dec 11, 2023

All type errors, documentation, and migration guide inconsistencies reported in this issue have since been fixed.
Did not get a response after asking for logs.

@FazilMuhammed getDeviceState has been removed in v5.x.x. Here is how you can get these values:

  1. externalId -> this is set by you, but we will soon add a getter for the externalId of the current user in the SDK. Look out for our next couple of releases.
  2. player ID -> this has been replaced with subscription ID (representing the push subscription) and can be retrieved via const id: string = OneSignal.User.pushSubscription.getPushSubscriptionId()
  3. onesignal ID is a new identifier that will represent a user, who can have multiple subscriptions. We will also add a getter for the onesignalID in a release soon.

@nan-li nan-li closed this as completed Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants