Skip to content

Commit 4d9f743

Browse files
hyochanclaude
hyochan
and
claude
authored
fix(listeners): use singleton native listener to prevent iOS removeAll() bug (#3164)
## Summary - Fix critical bug where iOS `removePurchaseUpdatedListener` called `removeAll()` instead of removing only the specific listener, silently wiping ALL registered listeners - Replace per-listener native registration with singleton native handler + JS-level `Set` fan-out for all 5 listener types - `remove()` now only deletes from the JS Set — other listeners remain intact regardless of iOS native behavior ## Root Cause When multiple `useIAP` hooks were active (e.g., a persistent top-level component + a screen-level component), unmounting one component called `removePurchaseUpdatedListener` on iOS which triggered `purchaseUpdatedListeners.removeAll()`. This wiped **all** listeners including the ones from the still-mounted component, causing `onPurchaseSuccess` to silently stop firing. Android was unaffected because it correctly used `.remove(listener)` instead of `.removeAll()`. ### Before (broken) ``` Component A: useIAP → registers listener A Component B: useIAP → registers listener B Component B unmounts → iOS removeAll() → listener A also gone ❌ User purchases → no listener to receive → purchase lost ``` ### After (fixed) ``` Component A: useIAP → adds cbA to JS Set Component B: useIAP → adds cbB to JS Set (native singleton already attached, no duplicate registration) Component B unmounts → Set.delete(cbB) → cbA still in Set ✅ User purchases → native singleton → fans out to cbA ✅ ``` ## Changes ### `src/index.ts` - Replace `WeakMap` per-listener tracking with module-level `Set` per event type - Register a single native handler that fans out to all JS listeners in the Set - `remove()` only deletes from JS Set (never calls native remove) - Add `resetListenerState()` called in `endConnection()` for clean re-registration - Applied to all 5 listener types: `purchaseUpdated`, `purchaseError`, `promotedProduct`, `userChoiceBilling`, `developerProvidedBilling` ### `src/__tests__/index.test.ts` - Update tests to verify singleton native registration (1 call, not N) - Verify JS-level removal: listener stops receiving after `remove()` - Verify independent removal: removing one listener doesn't affect others ## Test plan - [x] `yarn typecheck` passes - [x] `yarn lint` passes - [x] `yarn jest` — 251 tests pass Closes #3150 🤖 Generated with [Claude Code](https://claude.ai/code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a public way to fully clear and reset event-listener state to ensure clean reconnections. * **Refactor** * Centralized event dispatch so a single native callback fans out to multiple JS listeners for consistent broadcasting and error handling. * **Tests** * Updated tests to validate single-handler broadcasting, JS-level removal behavior, reconnection/re-registration, and normalized error forwarding. * **Documentation** * Clarified PR review workflow to require explicit POST when replying to individual review comments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e2acf36 commit 4d9f743

File tree

3 files changed

+259
-211
lines changed

3 files changed

+259
-211
lines changed

‎.claude/commands/review-pr.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ Fixed in abc1234 along with other review items.
8585
7. Push changes
8686

8787
8. Reply to **each individual review comment** using the comment's `id`:
88+
8889
```bash
89-
gh api repos/{owner}/{repo}/pulls/comments/{comment_id}/replies -f body="Fixed in abc1234."
90+
gh api repos/{owner}/{repo}/pulls/comments/{comment_id}/replies -X POST -f body="Fixed in abc1234."
9091
```
91-
**IMPORTANT:** Always reply directly to individual comments, NOT as a general PR review comment. Use the `/pulls/comments/{id}/replies` endpoint, NOT `gh pr review --comment`.
92+
93+
**CRITICAL:** You MUST include `-X POST` — without it the request defaults to GET and returns 404. Always reply directly to individual comments, NOT as a general PR review comment. Use the `/pulls/comments/{id}/replies` endpoint, NOT `gh pr review --comment`.

‎src/__tests__/index.test.ts‎

Lines changed: 59 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ describe('Public API (src/index.ts)', () => {
130130
const sub = IAP.purchaseUpdatedListener(listener);
131131
expect(typeof sub.remove).toBe('function');
132132

133-
// Emulate native event
133+
// Emulate native event via singleton handler
134134
const nitroPurchase = {
135135
id: 't1',
136136
productId: 'p1',
@@ -140,18 +140,23 @@ describe('Public API (src/index.ts)', () => {
140140
purchaseState: 'purchased',
141141
isAutoRenewing: false,
142142
};
143-
const wrapped = mockIap.addPurchaseUpdatedListener.mock.calls[0][0];
144-
wrapped(nitroPurchase);
143+
// Singleton: only one native handler registered
144+
expect(mockIap.addPurchaseUpdatedListener).toHaveBeenCalledTimes(1);
145+
const nativeHandler = mockIap.addPurchaseUpdatedListener.mock.calls[0][0];
146+
nativeHandler(nitroPurchase);
145147
expect(listener).toHaveBeenCalledWith(
146148
expect.objectContaining({
147149
productId: 'p1',
148150
platform: PLATFORM_IOS,
149151
}),
150152
);
151153

152-
// remove
154+
// remove only removes from JS set, not native
153155
sub.remove();
154-
expect(mockIap.removePurchaseUpdatedListener).toHaveBeenCalled();
156+
// Verify listener no longer fires after removal
157+
listener.mockClear();
158+
nativeHandler(nitroPurchase);
159+
expect(listener).not.toHaveBeenCalled();
155160
});
156161

157162
it('purchaseErrorListener forwards error objects and supports removal', () => {
@@ -160,8 +165,9 @@ describe('Public API (src/index.ts)', () => {
160165
expect(typeof sub.remove).toBe('function');
161166

162167
const err = {code: 'E_UNKNOWN', message: 'oops'};
163-
const passed = mockIap.addPurchaseErrorListener.mock.calls[0][0];
164-
passed(err);
168+
expect(mockIap.addPurchaseErrorListener).toHaveBeenCalledTimes(1);
169+
const nativeHandler = mockIap.addPurchaseErrorListener.mock.calls[0][0];
170+
nativeHandler(err);
165171
expect(listener).toHaveBeenCalledWith(
166172
expect.objectContaining({
167173
code: ErrorCode.Unknown,
@@ -170,7 +176,10 @@ describe('Public API (src/index.ts)', () => {
170176
);
171177

172178
sub.remove();
173-
expect(mockIap.removePurchaseErrorListener).toHaveBeenCalled();
179+
// Verify listener no longer fires after removal
180+
listener.mockClear();
181+
nativeHandler(err);
182+
expect(listener).not.toHaveBeenCalled();
174183
});
175184

176185
it('promotedProductListenerIOS warns and no-ops on non‑iOS', () => {
@@ -198,13 +207,18 @@ describe('Public API (src/index.ts)', () => {
198207
};
199208
const listener = jest.fn();
200209
const sub = IAP.promotedProductListenerIOS(listener);
201-
const wrapped = mockIap.addPromotedProductListenerIOS.mock.calls[0][0];
202-
wrapped(nitroProduct);
210+
expect(mockIap.addPromotedProductListenerIOS).toHaveBeenCalledTimes(1);
211+
const nativeHandler =
212+
mockIap.addPromotedProductListenerIOS.mock.calls[0][0];
213+
nativeHandler(nitroProduct);
203214
expect(listener).toHaveBeenCalledWith(
204215
expect.objectContaining({id: 'sku1', platform: PLATFORM_IOS}),
205216
);
206217
sub.remove();
207-
expect(mockIap.removePromotedProductListenerIOS).toHaveBeenCalled();
218+
// Verify listener no longer fires after removal
219+
listener.mockClear();
220+
nativeHandler(nitroProduct);
221+
expect(listener).not.toHaveBeenCalled();
208222
});
209223

210224
it('purchaseUpdatedListener ignores invalid purchase payload', () => {
@@ -215,13 +229,14 @@ describe('Public API (src/index.ts)', () => {
215229
expect(listener).not.toHaveBeenCalled();
216230
});
217231

218-
it('multiple purchaseUpdatedListeners all receive events', () => {
232+
it('multiple purchaseUpdatedListeners all receive events from single native handler', () => {
219233
const listener1 = jest.fn();
220234
const listener2 = jest.fn();
221235
const sub1 = IAP.purchaseUpdatedListener(listener1);
222236
const sub2 = IAP.purchaseUpdatedListener(listener2);
223237

224-
expect(mockIap.addPurchaseUpdatedListener).toHaveBeenCalledTimes(2);
238+
// Singleton: only one native listener registered
239+
expect(mockIap.addPurchaseUpdatedListener).toHaveBeenCalledTimes(1);
225240

226241
const nitroPurchase = {
227242
id: 't1',
@@ -232,10 +247,9 @@ describe('Public API (src/index.ts)', () => {
232247
purchaseState: 'purchased',
233248
isAutoRenewing: false,
234249
};
235-
const wrapped1 = mockIap.addPurchaseUpdatedListener.mock.calls[0][0];
236-
const wrapped2 = mockIap.addPurchaseUpdatedListener.mock.calls[1][0];
237-
wrapped1(nitroPurchase);
238-
wrapped2(nitroPurchase);
250+
// Single native handler dispatches to all JS listeners
251+
const nativeHandler = mockIap.addPurchaseUpdatedListener.mock.calls[0][0];
252+
nativeHandler(nitroPurchase);
239253

240254
expect(listener1).toHaveBeenCalledTimes(1);
241255
expect(listener2).toHaveBeenCalledTimes(1);
@@ -248,12 +262,12 @@ describe('Public API (src/index.ts)', () => {
248262
const listener1 = jest.fn();
249263
const listener2 = jest.fn();
250264
const sub1 = IAP.purchaseUpdatedListener(listener1);
251-
const sub2 = IAP.purchaseUpdatedListener(listener2);
265+
IAP.purchaseUpdatedListener(listener2);
252266

267+
// Remove first listener
253268
sub1.remove();
254-
expect(mockIap.removePurchaseUpdatedListener).toHaveBeenCalledTimes(1);
255269

256-
const wrapped2 = mockIap.addPurchaseUpdatedListener.mock.calls[1][0];
270+
const nativeHandler = mockIap.addPurchaseUpdatedListener.mock.calls[0][0];
257271
const nitroPurchase = {
258272
id: 't2',
259273
productId: 'p2',
@@ -263,26 +277,24 @@ describe('Public API (src/index.ts)', () => {
263277
purchaseState: 'purchased',
264278
isAutoRenewing: false,
265279
};
266-
wrapped2(nitroPurchase);
280+
nativeHandler(nitroPurchase);
281+
// listener2 still receives events, listener1 does not
267282
expect(listener2).toHaveBeenCalledTimes(1);
268283
expect(listener1).not.toHaveBeenCalled();
269-
270-
sub2.remove();
271284
});
272285

273-
it('multiple purchaseErrorListeners all receive errors', () => {
286+
it('multiple purchaseErrorListeners all receive errors from single native handler', () => {
274287
const listener1 = jest.fn();
275288
const listener2 = jest.fn();
276289
const sub1 = IAP.purchaseErrorListener(listener1);
277290
const sub2 = IAP.purchaseErrorListener(listener2);
278291

279-
expect(mockIap.addPurchaseErrorListener).toHaveBeenCalledTimes(2);
292+
// Singleton: only one native listener registered
293+
expect(mockIap.addPurchaseErrorListener).toHaveBeenCalledTimes(1);
280294

281-
const wrapped1 = mockIap.addPurchaseErrorListener.mock.calls[0][0];
282-
const wrapped2 = mockIap.addPurchaseErrorListener.mock.calls[1][0];
295+
const nativeHandler = mockIap.addPurchaseErrorListener.mock.calls[0][0];
283296
const err = {code: 'user-cancelled', message: 'User cancelled'};
284-
wrapped1(err);
285-
wrapped2(err);
297+
nativeHandler(err);
286298

287299
expect(listener1).toHaveBeenCalledTimes(1);
288300
expect(listener2).toHaveBeenCalledTimes(1);
@@ -295,16 +307,14 @@ describe('Public API (src/index.ts)', () => {
295307
const listener1 = jest.fn();
296308
const listener2 = jest.fn();
297309
const sub1 = IAP.purchaseErrorListener(listener1);
298-
const sub2 = IAP.purchaseErrorListener(listener2);
310+
IAP.purchaseErrorListener(listener2);
299311

300312
sub1.remove();
301313

302-
const wrapped2 = mockIap.addPurchaseErrorListener.mock.calls[1][0];
303-
wrapped2({code: 'network-error', message: 'Network error'});
314+
const nativeHandler = mockIap.addPurchaseErrorListener.mock.calls[0][0];
315+
nativeHandler({code: 'network-error', message: 'Network error'});
304316
expect(listener2).toHaveBeenCalledTimes(1);
305317
expect(listener1).not.toHaveBeenCalled();
306-
307-
sub2.remove();
308318
});
309319
});
310320

@@ -322,9 +332,10 @@ describe('Public API (src/index.ts)', () => {
322332
const listener1 = jest.fn();
323333
const sub1 = IAP.purchaseUpdatedListener(listener1);
324334

325-
// Verify listener is registered
335+
// Verify singleton native listener is registered
326336
expect(mockIap.addPurchaseUpdatedListener).toHaveBeenCalledTimes(1);
327-
const wrapped1 = mockIap.addPurchaseUpdatedListener.mock.calls[0][0];
337+
const nativeHandler1 =
338+
mockIap.addPurchaseUpdatedListener.mock.calls[0][0];
328339

329340
// Simulate a purchase event — listener should fire
330341
const nitroPurchase = {
@@ -336,10 +347,10 @@ describe('Public API (src/index.ts)', () => {
336347
purchaseState: 'purchased',
337348
isAutoRenewing: false,
338349
};
339-
wrapped1(nitroPurchase);
350+
nativeHandler1(nitroPurchase);
340351
expect(listener1).toHaveBeenCalledTimes(1);
341352

342-
// 2. Disconnect and remove old listener
353+
// 2. Disconnect (endConnection resets listener state)
343354
sub1.remove();
344355
await IAP.endConnection();
345356

@@ -349,12 +360,13 @@ describe('Public API (src/index.ts)', () => {
349360
const listener2 = jest.fn();
350361
const sub2 = IAP.purchaseUpdatedListener(listener2);
351362

352-
// New listener should be registered with native
363+
// New singleton native listener should be registered after reset
353364
expect(mockIap.addPurchaseUpdatedListener).toHaveBeenCalledTimes(1);
354-
const wrapped2 = mockIap.addPurchaseUpdatedListener.mock.calls[0][0];
365+
const nativeHandler2 =
366+
mockIap.addPurchaseUpdatedListener.mock.calls[0][0];
355367

356368
// Simulate purchase event on new connection — new listener should fire
357-
wrapped2(nitroPurchase);
369+
nativeHandler2(nitroPurchase);
358370
expect(listener2).toHaveBeenCalledTimes(1);
359371
expect(listener2).toHaveBeenCalledWith(
360372
expect.objectContaining({productId: 'p1'}),
@@ -377,9 +389,9 @@ describe('Public API (src/index.ts)', () => {
377389
const sub2 = IAP.purchaseErrorListener(errorListener2);
378390

379391
expect(mockIap.addPurchaseErrorListener).toHaveBeenCalledTimes(1);
380-
const wrapped = mockIap.addPurchaseErrorListener.mock.calls[0][0];
392+
const nativeHandler = mockIap.addPurchaseErrorListener.mock.calls[0][0];
381393

382-
wrapped({code: 'user-cancelled', message: 'User cancelled'});
394+
nativeHandler({code: 'user-cancelled', message: 'User cancelled'});
383395
expect(errorListener2).toHaveBeenCalledTimes(1);
384396
expect(errorListener2).toHaveBeenCalledWith(
385397
expect.objectContaining({
@@ -1777,9 +1789,10 @@ describe('Public API (src/index.ts)', () => {
17771789
);
17781790

17791791
sub.remove();
1780-
expect(
1781-
mockIap.removeDeveloperProvidedBillingListenerAndroid,
1782-
).toHaveBeenCalled();
1792+
// Singleton pattern: native remove is not called, JS listener is removed from Set
1793+
listener.mockClear();
1794+
wrapped(details);
1795+
expect(listener).not.toHaveBeenCalled();
17831796
});
17841797
});
17851798

0 commit comments

Comments
 (0)