Skip to content

Add custom app group name property#245

Closed
jarrodparkes wants to merge 4 commits intoOneSignal:mainfrom
AryeoHQ:main
Closed

Add custom app group name property#245
jarrodparkes wants to merge 4 commits intoOneSignal:mainfrom
AryeoHQ:main

Conversation

@jarrodparkes
Copy link
Copy Markdown

Description

One Line Summary

Adds a plugin property appGroupName to customize the app group name in accordance with https://documentation.onesignal.com/docs/ios-sdk-setup#use-custom-app-group-name.

Details

Motivation

Why is this code change being made? Or what is the goal of this PR?
Provide functionality expected by OneSignal's iOS SDK.

Testing

Manual testing

I tested this change with my own Expo-based application on a real device, and I was able to successfully send and receive a push notification from OneSignal.

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have personally tested this on my device, or explained why that is not possible
  • I have tested this on the latest version of the plugin
  • I have tested this on both Android and iOS

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

@eaguad1337
Copy link
Copy Markdown

I just tested it and works. Thank you.
Please merge this.

@Echelpoel Echelpoel mentioned this pull request Jun 12, 2025
Comment thread package.json
"@expo/image-utils": "^0.3.22"
}
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline

Comment thread yarn.lock Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the lock file

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rgomezp i'd be happy to keep it, if it wasn't preventing me from utilizing it. when testing/building things with Expo 53 yesterday it kept failing cause I use npm and yarn wasn't found for this prepare script. As I understand it, the npm invocation should work with either package manager whereas yarn does not

Comment thread package.json
"build": "npm run lint && rm -rf build && tsc && cp -a support/serviceExtensionFiles build/support/",
"lint": "eslint . --ext .ts",
"prepare": "yarn run build"
"prepare": "npm run build"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's keep yarn

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rgomezp i'd be happy to keep it, if it wasn't preventing me from utilizing it. when testing/building things with Expo 53 yesterday it kept failing cause I use npm and yarn wasn't found for this prepare script. As I understand it, the npm invocation should work with either package manager whereas yarn does not

assert.ok(config.ios?.bundleIdentifier, "Missing 'ios.bundleIdentifier' in app config.")
config.extra = getEasManagedCredentialsConfigExtra(config as ExpoConfig);
return config;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be simplified. Also if they provide an appGroupName we don't check for bundleId. props is optional anyway

@serolgames
Copy link
Copy Markdown

That would be a perfect merge. I'm using the commit as package in my project and it solved the issue. I hope it gets accepted very soon !

@serolgames
Copy link
Copy Markdown

@rgomezp is it possible to merge this ? I think it can be clean code later. A lot of people have this issue at the moment

@rgomezp
Copy link
Copy Markdown
Contributor

rgomezp commented Jun 30, 2025

@jkasten2 cc

@kelmag
Copy link
Copy Markdown

kelmag commented Jul 10, 2025

please merge this

@sherwinski
Copy link
Copy Markdown
Contributor

Hey, thanks for making this contribution to the project. The team merged #270, which adds support for customizing the app group name and as a result, have closed this PR since it's no longer needed.

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 this pull request may close these issues.

6 participants