Skip to content

Commit b317da4

Browse files
fix: address PR review feedback
- Add explicit android:priority="10" on intent filter - Add utf-8 encoding to writeFileSync for generated Kotlin source - Add test for preserving pre-existing services in AndroidManifest - Exclude __tests__ from tsconfig to match existing JS test convention - Remove unnecessary comments Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c21ff84 commit b317da4

4 files changed

Lines changed: 31 additions & 21 deletions

File tree

__tests__/withAndroidPushNotifications.test.ts

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
import path from 'path';
22
import fs from 'fs';
33

4-
// Mock @expo/config-plugins so we don't need the full Expo runtime.
5-
// withDangerousMod and withAndroidManifest just invoke their callbacks
6-
// immediately with the config object, simulating what Expo does at prebuild.
74
jest.mock('@expo/config-plugins', () => ({
85
withDangerousMod: (config: any, [_platform, callback]: [string, Function]) =>
96
callback(config),
@@ -18,10 +15,6 @@ jest.mock('@expo/config-plugins', () => ({
1815

1916
import { withAndroidPushNotifications } from '../src/expo-plugins/withAndroidPushNotifications';
2017

21-
/**
22-
* Helper to create a minimal Expo config object that the plugins expect.
23-
* Mirrors the shape that Expo passes during prebuild.
24-
*/
2518
function createMockConfig(packageName?: string) {
2619
return {
2720
name: 'TestApp',
@@ -91,21 +84,16 @@ describe('withAndroidPushNotifications', () => {
9184

9285
const content = writeFileSyncSpy.mock.calls[0][1] as string;
9386

94-
// Token forwarding
9587
expect(content).toContain(
9688
'IntercomModule.sendTokenToIntercom(application, refreshedToken)'
9789
);
98-
// Message filtering
9990
expect(content).toContain(
10091
'IntercomModule.isIntercomPush(remoteMessage)'
10192
);
102-
// Intercom message handling
10393
expect(content).toContain(
10494
'IntercomModule.handleRemotePushMessage(application, remoteMessage)'
10595
);
106-
// Non-Intercom passthrough
10796
expect(content).toContain('super.onMessageReceived(remoteMessage)');
108-
// Token passthrough
10997
expect(content).toContain('super.onNewToken(refreshedToken)');
11098
});
11199

@@ -147,7 +135,8 @@ describe('withAndroidPushNotifications', () => {
147135
});
148136
expect(writeFileSyncSpy).toHaveBeenCalledWith(
149137
path.join(expectedDir, 'IntercomFirebaseMessagingService.kt'),
150-
expect.any(String)
138+
expect.any(String),
139+
'utf-8'
151140
);
152141
});
153142
});
@@ -167,7 +156,7 @@ describe('withAndroidPushNotifications', () => {
167156
expect(service.$['android:exported']).toBe('false');
168157
});
169158

170-
test('registers MESSAGING_EVENT intent filter', () => {
159+
test('registers MESSAGING_EVENT intent filter with priority', () => {
171160
const config = createMockConfig('com.example.myapp');
172161
withAndroidPushNotifications(config as any, {} as any);
173162

@@ -178,12 +167,32 @@ describe('withAndroidPushNotifications', () => {
178167
expect(action.$['android:name']).toBe(
179168
'com.google.firebase.MESSAGING_EVENT'
180169
);
170+
expect(intentFilter.$['android:priority']).toBe('10');
171+
});
172+
173+
test('preserves existing services when adding Intercom service', () => {
174+
const config = createMockConfig('com.example.myapp');
175+
176+
config.modResults.manifest.application[0].service.push({
177+
$: {
178+
'android:name': '.SomeOtherService',
179+
'android:exported': 'false',
180+
},
181+
} as any);
182+
183+
withAndroidPushNotifications(config as any, {} as any);
184+
185+
const services = config.modResults.manifest.application[0].service;
186+
expect(services).toHaveLength(2);
187+
expect(services[0].$['android:name']).toBe('.SomeOtherService');
188+
expect(services[1].$['android:name']).toBe(
189+
'.IntercomFirebaseMessagingService'
190+
);
181191
});
182192

183193
test('does not duplicate service on repeated runs (idempotency)', () => {
184194
const config = createMockConfig('com.example.myapp');
185195

186-
// Run plugin twice on the same config
187196
withAndroidPushNotifications(config as any, {} as any);
188197
withAndroidPushNotifications(config as any, {} as any);
189198

@@ -194,7 +203,7 @@ describe('withAndroidPushNotifications', () => {
194203

195204
describe('error handling', () => {
196205
test('throws if android.package is not defined', () => {
197-
const config = createMockConfig(); // no package name
206+
const config = createMockConfig();
198207

199208
expect(() => {
200209
withAndroidPushNotifications(config as any, {} as any);

src/expo-plugins/withAndroidPushNotifications.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ const writeFirebaseService: ConfigPlugin<IntercomPluginProps> = (_config) =>
7070
fs.mkdirSync(serviceDir, { recursive: true });
7171
fs.writeFileSync(
7272
path.join(serviceDir, `${SERVICE_CLASS_NAME}.kt`),
73-
generateFirebaseServiceKotlin(packageName)
73+
generateFirebaseServiceKotlin(packageName),
74+
'utf-8'
7475
);
7576

7677
return config;
@@ -97,7 +98,6 @@ const registerServiceInManifest: ConfigPlugin<IntercomPluginProps> = (
9798

9899
const serviceName = `.${SERVICE_CLASS_NAME}`;
99100

100-
// Check if the service is already registered (idempotency)
101101
const existingService = mainApplication.service?.find(
102102
(s) => s.$?.['android:name'] === serviceName
103103
);
@@ -114,6 +114,9 @@ const registerServiceInManifest: ConfigPlugin<IntercomPluginProps> = (
114114
},
115115
'intent-filter': [
116116
{
117+
$: {
118+
'android:priority': '10',
119+
} as any,
117120
action: [
118121
{
119122
$: {

src/expo-plugins/withPushNotifications.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,8 @@ export const withIntercomPushNotification: ConfigPlugin<IntercomPluginProps> = (
6060
props
6161
) => {
6262
let newConfig = config;
63-
// iOS push notification setup
6463
newConfig = appDelegate(newConfig, props);
6564
newConfig = infoPlist(newConfig, props);
66-
// Android push notification setup
6765
newConfig = withAndroidPushNotifications(newConfig, props);
6866
return newConfig;
6967
};

tsconfig.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,5 @@
2525
"strict": true,
2626
"target": "esnext"
2727
},
28-
"exclude": ["examples"]
28+
"exclude": ["examples", "__tests__"]
2929
}

0 commit comments

Comments
 (0)