[FIX] Fixed RumSessionStarted listener implementation#839
Conversation
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
| return; | ||
| } | ||
|
|
||
| eventEmitter.addListener(RUM_SESSION_STARTED_EVENT_KEY, (event: any) => { |
There was a problem hiding this comment.
⚪ Code Quality Violation
Unexpected any. Specify a different type. (...read more)
Do not use the any type, as it is too broad and can lead to unexpected behavior.
There was a problem hiding this comment.
The event argument is defined as any in ReactNative NativeEventEmitter. Reference: https://github.com/facebook/react-native/blob/2be407cd9764617a71ea01cb441a068ce5c3068a/packages/react-native/Libraries/EventEmitter/NativeEventEmitter.d.ts#L55
|
|
||
| it('M registerRumSessionIdListener correctly registers a listener for RumSessionStarted', () => { | ||
| // GIVEN | ||
| const { registerRumSessionIdListener } = require('../sessionIdHelper'); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
There was a problem hiding this comment.
We have to use require statements in these tests, to be able to use jest.resetModules() to restore the initial state of the modules.
| it('M createNativeEventEmitterForModule initializes NativeEventEmitter only once W { platform = Android }', () => { | ||
| // GIVEN | ||
| mockPlatform('android'); | ||
| const createNativeEventEmitterForModule = require('../EventEmitter') |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
There was a problem hiding this comment.
We have to use require statements in these tests, to be able to use jest.resetModules() to restore the initial state of the modules.
| const { | ||
| registerRumSessionIdListener, | ||
| removeRumSessionIdListeners | ||
| } = require('../sessionIdHelper'); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
There was a problem hiding this comment.
We have to use require statements in these tests, to be able to use jest.resetModules() to restore the initial state of the modules.
marco-saia-datadog
force-pushed
the
marcosaia/fix/native-event-emitter
branch
from
4776906 to
8fe4cf7
Compare
| const { | ||
| setCachedRumSessionId, | ||
| getCachedRumSessionId | ||
| } = require('../sessionIdHelper'); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
There was a problem hiding this comment.
We have to use require statements in these tests, to be able to use jest.resetModules() to restore the initial state of the modules.
| it('M createNativeEventEmitterForModule returns valid event emitter W { platform = Android, NativeModules = undefined }', () => { | ||
| // GIVEN | ||
| mockPlatform('android'); | ||
| const createNativeEventEmitterForModule = require('../EventEmitter') |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
There was a problem hiding this comment.
We have to use require statements in these tests, to be able to use jest.resetModules() to restore the initial state of the modules.
|
|
||
| it('M registerRumSessionIdListener does not register more than one listener for RumSessionStarted', () => { | ||
| // GIVEN | ||
| const { registerRumSessionIdListener } = require('../sessionIdHelper'); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
There was a problem hiding this comment.
We have to use require statements in these tests, to be able to use jest.resetModules() to restore the initial state of the modules.
marco-saia-datadog
force-pushed
the
marcosaia/fix/native-event-emitter
branch
from
5c49257 to
511c41b
Compare
| it('M createNativeEventEmitterForModule returns valid event emitter W { platform = iOS, NativeModules = undefined }', () => { | ||
| // GIVEN | ||
| mockPlatform('ios'); | ||
| const createNativeEventEmitterForModule = require('../EventEmitter') |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
There was a problem hiding this comment.
We have to use require statements in these tests, to be able to use jest.resetModules() to restore the initial state of the modules.
marco-saia-datadog
force-pushed
the
marcosaia/fix/native-event-emitter
branch
from
d46cdfa to
a0afcce
Compare
| it('M createNativeEventEmitterForModule returns valid event emitter W { platform = Android, NativeModules != undefined }', () => { | ||
| // GIVEN | ||
| mockPlatform('android'); | ||
| const createNativeEventEmitterForModule = require('../EventEmitter') |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
There was a problem hiding this comment.
We have to use require statements in these tests, to be able to use jest.resetModules() to restore the initial state of the modules.
| const { | ||
| registerRumSessionIdListener, | ||
| getCachedRumSessionId | ||
| } = require('../sessionIdHelper'); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
There was a problem hiding this comment.
We have to use require statements in these tests, to be able to use jest.resetModules() to restore the initial state of the modules.
marco-saia-datadog
force-pushed
the
marcosaia/fix/native-event-emitter
branch
from
a0afcce to
cb3722b
Compare
3 participants
What does this PR do?
Fixes the
RumSessionStartednative event listener implementation.What changed
NativeModules.DdSdkisundefinedandNativeEventEmitterconstructor requires a non-null argument (ISSUE: Invariant Violation:new NativeEventEmitter()requires a non-null argument. #832)DdRum.getCurrentSessionId()will overwrite the cached RUM Session ID.Review checklist (to be filled by reviewers)