-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[notifications] Add support for custom managed workflow #12465
[notifications] Add support for custom managed workflow #12465
Conversation
@@ -30,7 +30,8 @@ export default async function getExpoPushTokenAsync(options: Options = {}): Prom | |||
|
|||
const deviceId = options.deviceId || (await getDeviceIdAsync()); | |||
|
|||
const experienceId = options.experienceId || (Constants.manifest && Constants.manifest.id); | |||
const experienceId = | |||
options.experienceId || Constants.manifest?.currentFullName || Constants.manifest?.id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just in case there are any differences introduced in Constants.manifest?.currentFullName
, shouldn't we change this to
options.experienceId || Constants.manifest?.id || Constants.manifest?.currentFullName;
so on the off chance there's a bug with Constants.manifest?.currentFullName
, we don't change existing users' push tokens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added my thoughts to the universe PR, linked below.
* Added support for currentFullName * Updated docs * Update CHANGELOG.md
Why
The bare workflow must support notifications out of the box in order to use managed EAS build. This PR implements support for bare workflow after landing this expo/config change and updating expo-constants expo/expo-cli#3376
How
Use
currentFullName || id
instead ofid
to get the legacy managed project id.Test Plan