Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip creation of EventManager if module not loaded #1427

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

srascar-bubble
Copy link

@srascar-bubble srascar-bubble commented Aug 31, 2022

Description

One Line Summary

Prevent the creation of a NativeEventEmitter with null as argument

Details

Creating EventManager when the module is not loaded will generate the following error

Invariant Violation: `new NativeEventEmitter()` requires a non-null argument.

Motivation

This change will affect most likely expo users.
Since version 4.x of OneSignal, EventManager is always created which leads to the error above
when running the app in Expo go and IOS Simulator.

Manual testing

  • Create a simple expo project
  • Import OneSignal and call OneSignal.init()
  • Launch ExpoGo in an IOS simulator

This change is Reviewable

This change will affect most likely `expo` users.
Since version 4.x of OneSignal, `EventManager` is always created which leads to the following error
when running the app in Expo go and IOS Simulator.

```
Invariant Violation: `new NativeEventEmitter()` requires a non-null argument.
```
@erickpiazza
Copy link

when this PR will be merged, some prediction I have the same problem

@srascar-bubble
Copy link
Author

diff --git a/node_modules/react-native-onesignal/dist/index.js b/node_modules/react-native-onesignal/dist/index.js
index 0958bde..74573a5 100644
--- a/node_modules/react-native-onesignal/dist/index.js
+++ b/node_modules/react-native-onesignal/dist/index.js
@@ -47,7 +47,12 @@ var NotificationReceivedEvent_1 = __importDefault(require("./events/Notification
 exports.NotificationReceivedEvent = NotificationReceivedEvent_1.default;
 var helpers_1 = require("./helpers");
 var RNOneSignal = react_native_1.NativeModules.OneSignal;
-var eventManager = new EventManager_1.default(RNOneSignal);
+var eventManager;
+
+if ((0, helpers_1.isNativeModuleLoaded)(RNOneSignal)) {
+  eventManager = new EventManager_1.default(RNOneSignal);
+}
+
 var OneSignal = /** @class */ (function () {
     function OneSignal() {
     }
diff --git a/node_modules/react-native-onesignal/src/index.ts b/node_modules/react-native-onesignal/src/index.ts
index 561dc0a..d57a4aa 100644
--- a/node_modules/react-native-onesignal/src/index.ts
+++ b/node_modules/react-native-onesignal/src/index.ts
@@ -30,7 +30,11 @@ import { InAppMessage, InAppMessageAction, InAppMessageLifecycleHandlerObject }
 import { isValidCallback, isNativeModuleLoaded } from './helpers';

 const RNOneSignal = NativeModules.OneSignal;
-const eventManager = new EventManager(RNOneSignal);
+let eventManager: EventManager;
+
+if (isNativeModuleLoaded(RNOneSignal)) {
+  eventManager = new EventManager(RNOneSignal);
+}

 // 0 = None, 1 = Fatal, 2 = Errors, 3 = Warnings, 4 = Info, 5 = Debug, 6 = Verbose
 export type LogLevel = 0 | 1 | 2 | 3 | 4 | 5 | 6;

@srascar-bubble
Copy link
Author

srascar-bubble commented Oct 25, 2022

@erickpiazza for the time being we apply the above patch

react-native-onesignal+4.4.1.patch

@delki8
Copy link

delki8 commented Nov 22, 2024

I guess this also fixes #1361. Two years and two approvals later, why hasn't this been merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants