As of this differential, when CommNotificationsHandler receives notification in foreground it sends it as broadcast. CommAndroidNotificationsEventEmitter receives the broadcast, parses notification into JavaScript-readable object and sends it as event to JavaScript. On the JavaScript side we
types data that we receive as event, removed no-op if statement checking for rescind and subscribed to event in push-handler.js
Details
Details
Build the app. Send notifications when app is in the foreground but target thread is not opened. Ensure notifications are displayed as they were previously.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/push/android.js | ||
|---|---|---|
| 28–35 | We can be sure since in D6273 we implement Java code that doesn't forward notification to JavaScript if it is a rescind. I had a discussion with @tomek about this. After D6229 this if statement does not do anything apart from calling invariant that kills the app if rescind without rescindID is received (we used to update redux previously) - therefore in the case of a correct rescind it is a no-op. We came to a conclusion with @tomek, that killing the app when receiving incomplete notification might not be desirable since in fact it is a server error. Taking this into account and the fact that in the case of correct rescind it is a no-op, it might be better just not to forward rescinds to JavaScript. When CommNotificationsHandler receives the rescinds it looks at its data, updates notification center, SQLite and returns. Only notifications with displayable content are forwarded to JavaScript (D6273). | |
| 45–46 | They must not be modified, therefore it makes logical sense to put them in $ReadOnly. I will check whether it can be simply done. | |
| native/push/android.js | ||
|---|---|---|
| 28–35 | Makes sense, thanks for explaining | |
Comment Actions
It seems like something is missing in this sentence
On the JavaScript side we types data that we receive as event, removed no-op if statement checking for rescind and subscribed to event in push-handler.js
| native/push/android.js | ||
|---|---|---|
| 14–15 | Why one of them is optional and one is mandatory but nullable? | |
| 46 | Why it is a tuple with one element instead of just the element? | |
| native/push/android.js | ||
|---|---|---|
| 14–15 | This the typing that satisfies types of JavaScript methods that parse notification content once it is visible in JavaScript. mergePrefixIntoBody and saveMessageInfos in push-handler in particular. | |
| 46 | When I remove those brackets and run flow in native directory I receive the following error: roperty @@iterator is missing in AndroidForegroundMessage [1] but exists in $Iterable [2]. [prop-missing]
../node_modules/react-native/Libraries/EventEmitter/NativeEventEmitter.js
73│
74│ addListener<TEvent: $Keys<TEventToArgsMap>>(
75│ eventType: TEvent,
76│ listener: (...args: $ElementType<TEventToArgsMap, TEvent>) => mixed,
77│ context?: mixed,
78│ ): EventSubscription {
79│ this._nativeModule?.addListener(eventType);
/private/tmp/flow/flowlib_2fd0d9f42c28b9ff_501/core.js
[2] 1650│ interface $Iterable<+Yield,+Return,-Next> {
push/android.js
[1] 54│ commAndroidNotificationsForegroundMessage: AndroidForegroundMessage,
Found 1 errorI might understand those types wrong but I think addListener of NativeEventEmitter is expected to take an array of arguments and those arrays of arguments are events emitted to to listeners. | |