feat(messaging): migrate FCM Web quickstart to FID-based APIs - #1080
feat(messaging): migrate FCM Web quickstart to FID-based APIs#1080hozhng wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the Firebase Cloud Messaging (FCM) Quickstart sample to use the latest Firebase SDK (v12.17.1) and transitions the implementation from legacy registration tokens to the new FID-based (Firebase Installation ID) APIs. Key changes include updating the main application and service worker scripts, revising the HTML UI to display the FID, and updating the README with the FCM HTTP v1 API documentation. The review feedback highlights several critical issues: a version mismatch between the service worker and the main application, potential runtime crashes in environments lacking the Notification API, a UI bug where denied permission errors are hidden, and a typo in the HTML charset attribute.
9bfe127 to
f439ad6
Compare
14de063 to
f7f3feb
Compare
Doris-Ge
left a comment
There was a problem hiding this comment.
Could you deploy your latest changes in this PR to https://android-gcm-test-519bd.web.app/?
f7f3feb to
3f12cd8
Compare
done! |
- Replace deprecated getToken/deleteToken with register/unregister and onRegistered/onUnregistered callbacks - Update UI elements and terminology from Registration Token to Installation ID (FID) - Upgrade firebase dependency to ^12.17.1 to support FID APIs - Update README documentation and cURL instructions for FCM HTTP v1 API Bug: b/522441204
3f12cd8 to
3da73f4
Compare
| class="mdl-card mdl-shadow--2dp mdl-cell mdl-cell--12-col mdl-cell--12-col-tablet mdl-cell--8-col-desktop" | ||
| > | ||
| <div class="mdl-card__supporting-text mdl-color-text--grey-600"> | ||
| <!-- div to display the generated registration FID --> |
There was a problem hiding this comment.
| <!-- div to display the generated registration FID --> | |
| <!-- div to display the registered FID --> |
| // TODO(developer): Retrieve a registration token for use with FCM. | ||
| // In many cases once an app has been granted notification permission, | ||
| // it should update its UI reflecting this. | ||
| // Retrieve registration FID for use with FCM. |
There was a problem hiding this comment.
| // TODO(developer): Retrieve a registration token for use with FCM. | |
| // In many cases once an app has been granted notification permission, | |
| // it should update its UI reflecting this. | |
| // Retrieve registration FID for use with FCM. | |
| // TODO(developer): Retrieve Firebase Installation ID (FID) for use with FCM. | |
| // In many cases once an app has been granted notification permission, | |
| // it should update its UI reflecting this. |
| console.log('Error retrieving registration token. ', err); | ||
| showToken('Error retrieving registration token.'); | ||
| function unregisterFromFirebase() { | ||
| // Unregister registration FID. |
There was a problem hiding this comment.
| // Unregister registration FID. | |
| // Unregister FID with FCM. |
| }).catch((err) => { | ||
| console.log('Error retrieving registration token. ', err); | ||
| showToken('Error retrieving registration token.'); | ||
| function unregisterFromFirebase() { |
There was a problem hiding this comment.
nit: rename this to unregister() or unregisterFidFromFirebase()
| console.log('Token deleted.', currentToken); | ||
| setTokenSentToServer(false); | ||
| // Once token is deleted update UI. | ||
| resetUI(); |
There was a problem hiding this comment.
Previously, if we delete a token, we will call resetUI() which calls getToken to display a new token. Now, we will go back to the previous "Needs Permission" page even though the permission is granted, which could be confusing. I think we should consider updating the second "Firebase Installation ID" page to have two buttons, "Register" and "Unregister", on the same row. Only one button should be enabled at a time. When "Register" button is enabled, fidElement should be empty or a message like "NotRegistered".
If "Register" fails, then we should stay on the same page and just log a warning. So does "Unregister".
Description
Migrate Firebase Cloud Messaging (FCM) Web quickstart sample app to use the new FID-based SDK APIs.
Key Changes
getToken/deleteTokenwithregister/unregisterandonRegistered/onUnregisteredcallbacks inmessaging/main.ts.firebaseto^12.17.1.README.mdto reflect FID registration flow and FCM HTTP v1 cURL format.Verification
npm run dev).onRegisteredcallback receiving FID, ForegroundonMessagehandler, andunregisterflow.npm run build).