This differential itroduces a file with custom code that checks if all message specs that might generate notifications are implemented in C++ as well. In the case of missing C++ message spec an dalert is displayed. Additionally this
feature required us to alter MessageSpec type a little bit so that generatesNotifs field is now optional.
Details
Details
Build the app and launch it. Ensure alert is not displayed. Go to message-types-validator.js file and comment the check for generatesNotifs in message spec. Now an alert with 5 message types should be displayed.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| native/types/message-types-validator.js | ||
|---|---|---|
| 23–27 ↗ | (On Diff #27456) | As discussed offline, we could consider having a banner (displayed at the top of the app) instead telling if there are unimplemented specs. The benefits are:
|
| native/types/message-types-validator.js | ||
|---|---|---|
| 23–27 ↗ | (On Diff #27456) | Created a follow up task to do during the next bug bash: https://linear.app/comm/issue/ENG-4097/refactor-missing-message-spec-alert-into-more-developer-friendly |
| lib/shared/messages/message-spec.js | ||
|---|---|---|
| 113 | Should this have been modified to return PushType? It looks like all of the cases where we return undefined have been removed, but I may be missing something | |
| lib/shared/messages/message-spec.js | ||
|---|---|---|
| 113 | Changing to PushType results in flow error. We removed all cases when generatesNotifs method invariantly returned undefined. Other generatesNotifs methods might return undefined in some cases depending on the particular notification content. For example: Cannot assign Object.freeze(...) to reactionMessageSpec because undefined [1] is incompatible with literal union [2] in
type argument R [3] of the return value of property generatesNotifs. [incompatible-type-arg]
shared/messages/reaction-message-spec.js
213│ );
214│
215│ if (targetMessageInfo?.creatorID !== notifTargetUserID) {
[1] 216│ return undefined;
217│ }
218│ return action === 'add_reaction' ? pushTypes.NOTIF : pushTypes.RESCIND;
219│ },
/private/tmp/flow/flowlib_2fd0d9f42c28b9ff_501/core.js
[3] 1809│ declare class Promise<+R> {
shared/messages/message-spec.js
[2] 113│ ) => Promise<PushType>, | |
| lib/shared/messages/message-spec.js | ||
|---|---|---|
| 113 | Thanks for explaining! | |