Repository navigation
feat: Web SDK update for version 28.2.0-rc.6 - #185
Conversation
🔵 Tier A · Mergeable after minor fixes
Updates the Web SDK to 28.2.0-rc.6 and adds notification-open callbacks, a web initial-notification API, and the exported notification-open payload type. Push authentication now supports cookie-fallback sessions, and background notifications post after subscription callbacks with deduplication by title. The analytics tracking helper and its exports from the earlier commits have been removed. Latest changes: The newest commit bumps the release to rc.6, adds notification-tap APIs, reads cookie-fallback sessions for push authentication, deduplicates notification titles, and removes analytics tracking.
Fix with agent prompt### Issue 1
src/services/push.ts:32
**Scope notification-open listeners to their Push instance**
With two Push instances on the page, tapping a notification from one invokes the other instance's listeners too, potentially routing data from a different project to the wrong handler. Please keep the listener set on the instance that creates the notification.
### Issue 2
src/services/push.ts:912
**Isolate failures in notification-open listeners**
If one listener throws, `forEach` stops and the remaining registered listeners never receive the tap; the exception also escapes onto the host page. Please catch and report errors per listener so one handler cannot block the others.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.📂 Walkthrough · 7
✅ Fixed since the last review · 3
Reviewed the commits since |
![hansi-codes[bot]](https://avatars.githubusercontent.com/in/5046327?s=60&v=4){kind=small}
ArnabChatterjee20k
changed the title
| } | ||
|
|
||
| // Taps on the notifications this page shows, reported to every onNotificationOpened. | ||
| const openedListeners = new Set<(opened: PushNotificationOpened) => void>(); |
There was a problem hiding this comment.
Scope notification-open listeners to their Push instance
With two Push instances on the page, tapping a notification from one invokes the other instance's listeners too, potentially routing data from a different project to the wrong handler. Please keep the listener set on the instance that creates the notification.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/push.ts
Line: 32
Comment:
**Scope notification-open listeners to their Push instance**
With two Push instances on the page, tapping a notification from one invokes the other instance's listeners too, potentially routing data from a different project to the wrong handler. Please keep the listener set on the instance that creates the notification.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
| window.focus(); | ||
| notification.close(); | ||
| const opened = toOpened(message.topic, message.data); | ||
| openedListeners.forEach((listener) => listener(opened)); |
There was a problem hiding this comment.
Isolate failures in notification-open listeners
If one listener throws, forEach stops and the remaining registered listeners never receive the tap; the exception also escapes onto the host page. Please catch and report errors per listener so one handler cannot block the others.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/push.ts
Line: 912
Comment:
**Isolate failures in notification-open listeners**
If one listener throws, `forEach` stops and the remaining registered listeners never receive the tap; the exception also escapes onto the host page. Please catch and report errors per listener so one handler cannot block the others.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · error-handling · Reply if this doesn't apply.
1 participant
This PR contains updates to the Web SDK for version 28.2.0-rc.6.
What's Changed
Push.onNotificationOpened()fires when a user taps a shown notificationPush.getInitialNotification()(returnsnullon web) and thePushNotificationOpenedtypesubscribe()resolves the signed-in user fromcookieFallback, not just an explicit sessionRegenerated with sdk-generator 5.5.1.