feat(fcm): Enable fid and deprecate token for Send API#3145
feat(fcm): Enable fid and deprecate token for Send API#3145yvonnep165 wants to merge 13 commits into
fid and deprecate token for Send API#3145Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for Firebase Installation IDs (FIDs) in the messaging service by adding the FidMessage interface and updating the Message union type. The MulticastMessage interface now supports targeting via fids, and the tokens field has been deprecated and made optional. Feedback includes suggestions to add documentation for the new FidMessage interface, refactor the message construction logic using object destructuring for better maintainability, and improve the grammar of validation error messages. Additionally, the reviewer noted that making tokens optional is a breaking change for TypeScript consumers that should be highlighted in release notes.
…or the messages array construction in MulticastMessage
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for Firebase Installation IDs (FIDs) in the messaging service by adding FidMessage and FidMulticastMessage interfaces. It deprecates TokenMessage and MulticastMessage and updates the sendEachForMulticast method to handle both registration tokens and FIDs, ensuring the total count does not exceed the 500-item limit. Comprehensive unit and integration tests have been added to validate the new functionality and error handling. I have no feedback to provide.
| * This method uses the {@link Messaging.sendEach} API under the hood to send the given | ||
| * message to all the target recipients. The responses list obtained from the | ||
| * return value corresponds to the order of tokens in the `MulticastMessage`. | ||
| * return value corresponds to the order of tokens/fids in the `MulticastMessage`. |
There was a problem hiding this comment.
nit: Maybe clarify that tokens take precedence if both are provided.
| /** | ||
| * Payload for the {@link Messaging.send} operation. The payload contains all the fields | ||
| * in the BaseMessage type, and exactly one of token, topic or condition. | ||
| * in the BaseMessage type, and exactly one of fid, token, topic or condition. |
There was a problem hiding this comment.
Should we mention that token is deprecated here as well?
in the BaseMessage type, and exactly one of fid, token (deprecated, use fid instead), topic or condition.
This change deprecates
TokenMessageinterface, transitioning support toFidMessageinterface. A newFidMulticastMessageinterface containing only fids: string[] is added. An optionalfidsis added to the legacyMulticastMessageinterface, which is deprecated, to support mixed transition batches.The multicast helper function
sendEachForMulticastis updated to support both target types concurrently up to a combined total of 500. And function overloads are implemented onsendEachForMulticastto support both tokens and fids payloads.Note for API doc: Hyper-reference link {@link Messaging.sendEachForMulticast} doesn't work in api extraction since there are distinct method declarations due to function overloads. (throwing an
ae-unresolved-linkerror) . So I change it to plain comment text.