Skip to content

Commit 1af378b

Browse files
fix: add missing onPaywallClosed event. add internalHandlers for paywall to unsubscribe after onPaywallClosed
1 parent 3031d91 commit 1af378b

File tree

4 files changed

+128
-16
lines changed

4 files changed

+128
-16
lines changed

src/__tests__/integration/ui/paywall/paywall-view-controller-events.test.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,7 +1364,10 @@ describe('ViewController - dismiss cleanup', () => {
13641364
// Call dismiss
13651365
await view.dismiss();
13661366

1367-
// Emit events AFTER dismiss - handlers should NOT be called
1367+
// Emit onPaywallClosed event - this triggers cleanup via internal handler
1368+
emitPaywallViewDisappearedEvent(viewId, sample.view);
1369+
1370+
// Emit events AFTER cleanup - handlers should NOT be called
13681371
emitPaywallProductSelectedEvent(viewId, sample.product_id, sample.view);
13691372
emitPaywallPurchaseStartedEvent(
13701373
viewId,
@@ -1374,7 +1377,7 @@ describe('ViewController - dismiss cleanup', () => {
13741377
emitPaywallUserActionEvent(viewId, 'close', undefined, sample.view);
13751378
emitPaywallRestoreStartedEvent(viewId, sample.view);
13761379

1377-
// Verify that NONE of the handlers were called after dismiss
1380+
// Verify that NONE of the handlers were called after cleanup
13781381
expect(onProductSelectedHandler).not.toHaveBeenCalled();
13791382
expect(onPurchaseStartedHandler).not.toHaveBeenCalled();
13801383
expect(onCloseHandler).not.toHaveBeenCalled();
@@ -1657,7 +1660,12 @@ describe('ViewController - onPaywallClosed after user action close', () => {
16571660
});
16581661

16591662
// 1. User presses close button - this should trigger dismiss
1660-
emitPaywallUserActionEvent(viewId, 'close', undefined, closeButtonSample.view);
1663+
emitPaywallUserActionEvent(
1664+
viewId,
1665+
'close',
1666+
undefined,
1667+
closeButtonSample.view,
1668+
);
16611669

16621670
expect(onCloseButtonPressHandler).toHaveBeenCalledTimes(1);
16631671
expect(dismissSpy).toHaveBeenCalledTimes(1);
@@ -1668,8 +1676,8 @@ describe('ViewController - onPaywallClosed after user action close', () => {
16681676
// 3. Native code sends onPaywallClosed event after paywall actually closes
16691677
emitPaywallViewDisappearedEvent(viewId, closedSample.view);
16701678

1671-
// BUG EXPECTATION: onPaywallClosed handler should be called,
1672-
// but it might not be if dismiss() unsubscribed all handlers before native sent the event
1679+
// VERIFICATION: onPaywallClosed handler should be called
1680+
// Cleanup happens via internal handler AFTER this client handler executes
16731681
expect(onPaywallClosedHandler).toHaveBeenCalledTimes(1);
16741682

16751683
dismissSpy.mockRestore();

src/ui/view-controller.test.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ jest.mock('./view-emitter', () => {
2121
return {
2222
ViewEmitter: jest.fn().mockImplementation(() => ({
2323
addListener: jest.fn(),
24+
addInternalListener: jest.fn(),
2425
removeAllListeners: jest.fn(),
2526
})),
2627
};
@@ -127,7 +128,7 @@ describe('ViewController', () => {
127128
});
128129

129130
describe('dismiss', () => {
130-
it('calls bridge and unsubscribes listeners', async () => {
131+
it('calls bridge and registers internal cleanup handler', async () => {
131132
const { AdaptyPaywallCoder } = jest.requireMock(
132133
'@/coders/adapty-paywall',
133134
);
@@ -140,15 +141,23 @@ describe('ViewController', () => {
140141
.mockResolvedValueOnce(undefined); // dismiss
141142

142143
const { ViewEmitter } = jest.requireMock('./view-emitter');
144+
const addInternalListenerMock = jest.fn();
143145
const removeAllListenersMock = jest.fn();
144146
(ViewEmitter as unknown as jest.Mock).mockImplementation(() => ({
145147
addListener: jest.fn(),
148+
addInternalListener: addInternalListenerMock,
146149
removeAllListeners: removeAllListenersMock,
147150
}));
148151

149152
const view = await ViewController.create(paywall, {} as any);
150153
view.setEventHandlers({ onCloseButtonPress: () => true });
151154

155+
// Verify internal handler was registered for cleanup
156+
expect(addInternalListenerMock).toHaveBeenCalledWith(
157+
'onPaywallClosed',
158+
expect.any(Function),
159+
);
160+
152161
await view.dismiss();
153162

154163
expect($bridge.request).toHaveBeenLastCalledWith(
@@ -157,7 +166,10 @@ describe('ViewController', () => {
157166
'Void',
158167
expect.any(Object),
159168
);
160-
expect(removeAllListenersMock).toHaveBeenCalled();
169+
170+
// Cleanup now happens via internal handler, not directly in dismiss()
171+
// So removeAllListeners should NOT be called immediately after dismiss
172+
expect(removeAllListenersMock).not.toHaveBeenCalled();
161173
});
162174

163175
it('throws if id is null', async () => {
@@ -181,6 +193,7 @@ describe('ViewController', () => {
181193
const addListener = jest.fn();
182194
(ViewEmitter as unknown as jest.Mock).mockImplementation(() => ({
183195
addListener,
196+
addInternalListener: jest.fn(),
184197
removeAllListeners: jest.fn(),
185198
}));
186199

@@ -229,6 +242,7 @@ describe('ViewController', () => {
229242
// Mock ViewEmitter instance BEFORE create
230243
(ViewEmitter as unknown as jest.Mock).mockImplementation(() => ({
231244
addListener,
245+
addInternalListener: jest.fn(),
232246
removeAllListeners,
233247
}));
234248

@@ -292,6 +306,7 @@ describe('ViewController', () => {
292306
});
293307
(ViewEmitter as unknown as jest.Mock).mockImplementation(() => ({
294308
addListener,
309+
addInternalListener: jest.fn(),
295310
removeAllListeners: jest.fn(),
296311
}));
297312

@@ -347,6 +362,7 @@ describe('ViewController', () => {
347362
});
348363
(ViewEmitter as unknown as jest.Mock).mockImplementation(() => ({
349364
addListener,
365+
addInternalListener: jest.fn(),
350366
removeAllListeners: jest.fn(),
351367
}));
352368

@@ -401,6 +417,7 @@ describe('ViewController', () => {
401417
const addListener = jest.fn();
402418
(ViewEmitter as unknown as jest.Mock).mockImplementation(() => ({
403419
addListener,
420+
addInternalListener: jest.fn(),
404421
removeAllListeners: jest.fn(),
405422
}));
406423

src/ui/view-controller.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,18 @@ export class ViewController {
7272
);
7373

7474
view.id = result.id;
75+
view.viewEmitter = new ViewEmitter(result.id);
7576

7677
view.setEventHandlers(DEFAULT_EVENT_HANDLERS);
7778

79+
// Register internal handler for cleanup
80+
view.viewEmitter.addInternalListener('onPaywallClosed', () => {
81+
// Called AFTER client's onPaywallClosed handler
82+
if (view.viewEmitter) {
83+
view.viewEmitter.removeAllListeners();
84+
}
85+
});
86+
7887
return view;
7988
}
8089

@@ -184,10 +193,6 @@ export class ViewController {
184193
} satisfies Req['AdaptyUIDismissPaywallView.Request']);
185194

186195
await this.handle<void>(methodKey, body, 'Void', ctx, log);
187-
188-
if (this.viewEmitter) {
189-
this.viewEmitter.removeAllListeners();
190-
}
191196
}
192197

193198
/**

src/ui/view-emitter.ts

Lines changed: 87 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ export class ViewEmitter {
3636
onRequestClose: () => Promise<void>;
3737
}
3838
> = new Map();
39+
private internalHandlers: Map<
40+
EventName,
41+
{
42+
handler: (event: ParsedPaywallEvent) => void;
43+
config: (typeof HANDLER_TO_EVENT_CONFIG)[EventName];
44+
}
45+
> = new Map();
3946

4047
constructor(viewId: string) {
4148
this.viewId = viewId;
@@ -59,6 +66,7 @@ export class ViewEmitter {
5966
onRequestClose,
6067
});
6168

69+
// If no subscription to native event exists - create one
6270
if (!this.eventListeners.has(config.nativeEvent)) {
6371
const subscription = $bridge.addEventListener(
6472
config.nativeEvent,
@@ -70,6 +78,54 @@ export class ViewEmitter {
7078
return this.eventListeners.get(config.nativeEvent)!;
7179
}
7280

81+
/**
82+
* Adds an internal event handler.
83+
* Internal handlers:
84+
* - Are called AFTER client handlers
85+
* - Do NOT return boolean (don't affect auto-dismiss)
86+
* - Are used for internal SDK logic (e.g., cleanup)
87+
* @internal
88+
*/
89+
public addInternalListener(
90+
event: EventName,
91+
callback: (event: ParsedPaywallEvent) => void,
92+
): void {
93+
const config = HANDLER_TO_EVENT_CONFIG[event];
94+
95+
if (!config) {
96+
throw new Error(`No event config found for handler: ${event}`);
97+
}
98+
99+
// Replace existing internal handler for this event
100+
this.internalHandlers.set(event, {
101+
handler: callback,
102+
config,
103+
});
104+
105+
// If no subscription to native event exists - create one
106+
if (!this.eventListeners.has(config.nativeEvent)) {
107+
const subscription = $bridge.addEventListener(
108+
config.nativeEvent,
109+
this.createEventHandler(config),
110+
);
111+
this.eventListeners.set(config.nativeEvent, subscription);
112+
}
113+
}
114+
115+
/**
116+
* Checks if handler should be skipped based on action type filtering
117+
*/
118+
private shouldSkipHandler(
119+
handlerConfig: { propertyMap?: { [key: string]: string } },
120+
parsedEvent: ParsedPaywallEvent,
121+
): boolean {
122+
return !!(
123+
handlerConfig.propertyMap &&
124+
parsedEvent.id === PaywallEventId.DidPerformAction &&
125+
parsedEvent.action.type !== handlerConfig.propertyMap['action']
126+
);
127+
}
128+
73129
private createEventHandler(
74130
config: (typeof HANDLER_TO_EVENT_CONFIG)[EventName],
75131
) {
@@ -91,6 +147,7 @@ export class ViewEmitter {
91147
const possibleHandlers =
92148
NATIVE_EVENT_TO_HANDLERS[config.nativeEvent] || [];
93149

150+
// 1. Client handlers
94151
let hasError = false;
95152
for (const handlerName of possibleHandlers) {
96153
const handlerData = this.handlers.get(handlerName);
@@ -101,11 +158,7 @@ export class ViewEmitter {
101158
const { handler, config: handlerConfig, onRequestClose } = handlerData;
102159

103160
// Filter by action type for DidPerformAction events
104-
if (
105-
handlerConfig.propertyMap &&
106-
parsedEvent.id === PaywallEventId.DidPerformAction &&
107-
parsedEvent.action.type !== handlerConfig.propertyMap['action']
108-
) {
161+
if (this.shouldSkipHandler(handlerConfig, parsedEvent)) {
109162
continue;
110163
}
111164

@@ -141,6 +194,34 @@ export class ViewEmitter {
141194
}
142195
}
143196

197+
// 2. Internal handlers
198+
for (const handlerName of possibleHandlers) {
199+
const internalHandlerData = this.internalHandlers.get(handlerName);
200+
if (!internalHandlerData) {
201+
continue;
202+
}
203+
204+
const { handler, config: handlerConfig } = internalHandlerData;
205+
206+
// Use the same filtering logic
207+
if (this.shouldSkipHandler(handlerConfig, parsedEvent)) {
208+
continue;
209+
}
210+
211+
try {
212+
handler(parsedEvent);
213+
} catch (error) {
214+
// Log error but continue execution
215+
log.failed({
216+
error,
217+
handlerName: `internal:${handlerName}`,
218+
viewId: eventViewId,
219+
eventId: parsedEvent.id,
220+
reason: 'internal_handler_failed',
221+
});
222+
}
223+
}
224+
144225
if (!hasError) {
145226
log.success({ viewId: eventViewId, eventId: parsedEvent.id });
146227
}
@@ -151,6 +232,7 @@ export class ViewEmitter {
151232
this.eventListeners.forEach(subscription => subscription.remove());
152233
this.eventListeners.clear();
153234
this.handlers.clear();
235+
this.internalHandlers.clear();
154236
}
155237
}
156238

0 commit comments

Comments
 (0)