xmppjs / xmpp.js

XMPP for JavaScript
ISC License
2.19k stars 373 forks source link

EventEmitter related conflict with Spotify SDK #795

Closed sschimmel closed 4 years ago

sschimmel commented 4 years ago

So, I've got this wierd issue with a conflict between xmpp.js and a package called rn-spotify-sdk. I'm not entirely sure that the root cause is in xmpp.js but uninstalling xmpp.js fixes the error.

I have integrated xmpp.js in a React Native app and it's working fine. In the same app Spotify is also integrated through their native SDK's and a package called rn-spotify-sdk. Whenever I install @xmpp/client using yarn add @xmpp/client I get this error: Error: subscriber must be an EventEmitter. Full details are below in the screenshot.

Besides uninstalling xmpp.js the error also dissapears when I remove the import for the rn-spotify-sdk. That offcourse breaks my Spotify integration as is expected but the error is gone and the app for the rest works fine including sending and receiving messages using xmpp.

The error is in line 9 of Queue.js of the spotify sdk. The last line of this code snippet is line 9 from the error:

import EventEmitter from 'events';
import Spotify from './Spotify';
import RNEvents from 'react-native-events';

const Queue = new EventEmitter();
const nativeEvents = new EventEmitter();
RNEvents.addPreSubscriber(Spotify, nativeEvents);

The subscriber is the second parameter to addPreSubscriber which is nativeEvents and thats a EventEmitter;

The code from addPreSubscriber:

RNEvents.addPreSubscriber = (nativeModule, subscriber) => {
    if(!nativeModule.__registerAsJSEventEmitter) {
        throw new Error("Native module does not conform to RNEventConformer");
    }
    else if(nativeModule.__rnEventsId == null) {
        throw new Error("Native module has not been registered");
    }
    else if(!(subscriber instanceof EventEmitter)) {
        throw new Error("subscriber must be an EventEmitter");
    }
    const moduleId = nativeModule.__rnEventsId;
    const moduleInfo = registeredModules[moduleId];
    if(moduleInfo == null) {
        throw new Error("No module info found for native module");
    }

    // add subscriber
    moduleInfo.preSubscribers.push(subscriber);
}

The rn-spotify-sdk uses a package called [react-native-events](https://github.com/lufinkey/react-native-events) which is "A full EventEmitter implementation for react-native modules".

From their Readme:

In order for your native module to conform to node's EventEmitter class, you must first register your module as an event emitter inside your native module's index.js using the register method, and then conform it using the conform method:

import { NativeModules } from 'react-native';
import RNEvents from 'react-native-events';

var MyNativeModule = NativeModules.MyNativeModule;

// register your event emitter to be able to send events
RNEvents.register(MyNativeModule);
// conform your native module to EventEmitter
RNEvents.conform(MyNativeModule);

export default MyNativeModule;

The full error: afbeelding

The xmpp related code is still in a feature branch in my repository and switching to any other branch will fix this. Running yarn remove @xmpp/client in the feature branch will also fix the error.

Reading through the xmpp code I see that an EventEmitter is used but I didn't see anything in particular that would indicate a conflict between packages. Is there something special being done regarding the EventEmitter or is it being overwritten?

sschimmel commented 4 years ago

I've done some more research and it seems that xmpp.js changes the EventEmitter.

In queue.js from rn-spotify-sdk I added some console logs:

const Queue = new EventEmitter();
const nativeEvents = new EventEmitter();
console.log('In Spotify SDK');
console.log(nativeEvents);
console.log(EventEmitter);
console.log(nativeEvents instanceof EventEmitter)
RNEvents.addPreSubscriber(Spotify, nativeEvents);

And I also added some more console.logs to react-native-events: RNEvents.addPreSubscriber = (nativeModule, subscriber) => { console.log('IN React native events'); console.log("RN nativeModule: ", nativeModule); console.log("Subscriber: ", subscriber); console.log("EventEmitter: ", EventEmitter) console.log(subscriber instanceof EventEmitter)

if(!nativeModule.__registerAsJSEventEmitter) {
    throw new Error("Native module does not conform to RNEventConformer");
}
else if(nativeModule.__rnEventsId == null) {
    throw new Error("Native module has not been registered");
}
else if(!(subscriber instanceof EventEmitter)) {
    throw new Error("subscriber must be an EventEmitter");
}
const moduleId = nativeModule.__rnEventsId;
const moduleInfo = registeredModules[moduleId];
if(moduleInfo == null) {
    throw new Error("No module info found for native module");
}

// add subscriber
moduleInfo.preSubscribers.push(subscriber);

}

React native events checks if the subscriber is an instance of EventEmitter. As the subscriber nativeEvents is a new instance of EventEmitter that should be the case. Both are importing EventEmitter from events: import EventEmitter from 'events';

Now when we look at the output of the console.logs we see that the two instances are indeed different so the check returns false as it should.

In Spotify SDK
EventEmitter {_events: {…}, _eventsCount: 0, _maxListeners: undefined}
ƒ EventEmitter() {
    EventEmitter.init.call(this);
  }
true
IN React native events
RN nativeModule:  {__registerAsJSEventEmitter: ƒ, authenticate: ƒ, getPlaybackMetadata: ƒ, getPlaybackMetadataAsync: ƒ, getPlaybackState: ƒ, …}
Subscriber:  EventEmitter {_events: {…}, _eventsCount: 0, _maxListeners: undefined}
EventEmitter:  ƒ EventEmitter() {
    this._events = this._events || {};
    this._maxListeners = this._maxListeners || undefined;
  }
 false

In the dist folder of xmpp/client I found this snippet of code:

function EventEmitter() {
  if (!this._events || !Object.prototype.hasOwnProperty.call(this, '_events')) {
    this._events = objectCreate(null);
    this._eventsCount = 0;
  }

  this._maxListeners = this._maxListeners || undefined;
}

That is the same as the output from the second console.log(EventEmitter).

That got me digging further and the error seems to be in the versions used of the events package. React Native Events uses version 1.1.1 and xmppjs uses 3.0.0.

Not a problem for xmpp.js but for react-native-events to solve :-)

sonnyp commented 4 years ago

https://github.com/lufinkey/react-native-events/issues/5

I'm a bit confused.

In theory and as far as I know/remember; metro (the bundler for React Native) should replace all import of events with its own implementation. Not sure why you end up with multiple incompatible implementations.

sschimmel commented 4 years ago

If it is supposed to do that it might be versioned too? The problem was solved by adding this to our package.json:

"resolutions": {
    "events": "3.0.0"
  }
sonnyp commented 4 years ago

@sschimmel right.

Thanks!