WIP: Remove EventEmitter from roslib.js bundle

[Rayman]

I'm not sure why, but EventEmitter is included within roslib.js. It should be a dependency according to all the readmes. This change excludes it from the bundle.

name	roslib.js	roslib.min.js
old	131K	53K
new	104K	42K

Bundle size analysis here: https://roslibjs-bundle-size-example.netlify.com

[mvollrath]

The output roslib.js works, but this breaks npm run publish tests for me.

$ nodejs --version
v8.10.0
$ npm --version
6.5.0-next.0
$ rm -rf node_modules/
$ npm install
...
$ npm run publish

Running "karma:build" (karma) task
11 02 2020 15:49:48.504:INFO [karma-server]: Karma v3.1.4 server started at http://0.0.0.0:9876/
11 02 2020 15:49:48.506:INFO [launcher]: Launching browsers Firefox with concurrency unlimited
11 02 2020 15:49:48.509:INFO [launcher]: Starting browser Firefox
11 02 2020 15:49:50.184:INFO [Firefox 72.0.0 (Ubuntu 0.0.0)]: Connected on socket iJ8qYUNqg_-R4eRuAAAA with id 85713544
Firefox 72.0.0 (Ubuntu 0.0.0) ERROR
  {
    "message": "Error: Cannot find module 'eventemitter2'\nat /home/matt/src/roslibjs/build/roslib.js:1:159\n\no@/home/matt/src/roslibjs/build/roslib.js:1:159\no/<@/home/matt/src/roslibjs/build/roslib.js:1:316\n[14]<@/home/matt/src/roslibjs/build/roslib.js:1967:28\no@/home/matt/src/roslibjs/build/roslib.js:1:265\no/<@/home/matt/src/roslibjs/build/roslib.js:1:316\n[13]<@/home/matt/src/roslibjs/build/roslib.js:1276:22\no@/home/matt/src/roslibjs/build/roslib.js:1:265\no/<@/home/matt/src/roslibjs/build/roslib.js:1:316\n[19]<@/home/matt/src/roslibjs/build/roslib.js:2459:10\no@/home/matt/src/roslibjs/build/roslib.js:1:265\no/<@/home/matt/src/roslibjs/build/roslib.js:1:316\n[4]<@/home/matt/src/roslibjs/build/roslib.js:601:16\no@/home/matt/src/roslibjs/build/roslib.js:1:265\no/<@/home/matt/src/roslibjs/build/roslib.js:1:316\n[5]</<@/home/matt/src/roslibjs/build/roslib.js:615:17\n[5]<@/home/matt/src/roslibjs/build/roslib.js:616:4\no@/home/matt/src/roslibjs/build/roslib.js:1:265\nr@/home/matt/src/roslibjs/build/roslib.js:1:432\n@/home/matt/src/roslibjs/build/roslib.js:1:460\n",
    "str": "Error: Cannot find module 'eventemitter2'\nat /home/matt/src/roslibjs/build/roslib.js:1:159\n\no@/home/matt/src/roslibjs/build/roslib.js:1:159\no/<@/home/matt/src/roslibjs/build/roslib.js:1:316\n[14]<@/home/matt/src/roslibjs/build/roslib.js:1967:28\no@/home/matt/src/roslibjs/build/roslib.js:1:265\no/<@/home/matt/src/roslibjs/build/roslib.js:1:316\n[13]<@/home/matt/src/roslibjs/build/roslib.js:1276:22\no@/home/matt/src/roslibjs/build/roslib.js:1:265\no/<@/home/matt/src/roslibjs/build/roslib.js:1:316\n[19]<@/home/matt/src/roslibjs/build/roslib.js:2459:10\no@/home/matt/src/roslibjs/build/roslib.js:1:265\no/<@/home/matt/src/roslibjs/build/roslib.js:1:316\n[4]<@/home/matt/src/roslibjs/build/roslib.js:601:16\no@/home/matt/src/roslibjs/build/roslib.js:1:265\no/<@/home/matt/src/roslibjs/build/roslib.js:1:316\n[5]</<@/home/matt/src/roslibjs/build/roslib.js:615:17\n[5]<@/home/matt/src/roslibjs/build/roslib.js:616:4\no@/home/matt/src/roslibjs/build/roslib.js:1:265\nr@/home/matt/src/roslibjs/build/roslib.js:1:432\n@/home/matt/src/roslibjs/build/roslib.js:1:460\n"
  }

[Rayman]

You are right, I broke the tests. I've now fixed them.

I've to check webpack next to make sure I'm not breaking anybody's workflow.

[mvollrath]

It works now, but I'm not sure this should be done.

Removing EventEmitter from the bundle makes ROSLIB smaller, but you still have to obtain EventEmitter, so the overall application size is probably about the same and it's a separate HTTP request to a separate host rather than multiplexed. In my quick testing, this PR + going to a CDN for EventEmitter doubles the load time of the ROSLIB stack compared to using bundled EventEmitter on develop.

A better change might be to stop instructing people to load EventEmitter from CDN (or anywhere), because it's no longer required and is just slowing things down. Bundling the dependency is more convenient and faster for most users, I think.

[Rayman]

So the problem I have is that I'm also using EventEmitter in my own code and so the library will have to be loaded twice. This is wasted data.

With this PR I'm only having to download one copy. This is how it used to work before 5d1333750d836176ec89c42731e251f3b430476b.

[EzraBrooks]

Converting this PR to a draft, as the title indicates it's WIP.

[MatthijsBurgh]

@EzraBrooks I see you marked this as a draft. But what do you think of this PR? I think this is a relevant PR to consider, when we want to improve library quality.

I do know @Rayman, so I know he is open to discuss/explain when needed.

[EzraBrooks]

I definitely agree with this from a philosophical perspective, but won't it break the CDN-served scripts? Might require a major version bump.

[Rayman]

@MatthijsBurgh in this PR https://github.com/RobotWebTools/roslibjs/pull/615 you removed loading eventemitter from the CDN. This means the bundled eventemitter is now used. So this PR will break the examples.

Originally this package was designed that you first load eventemitter2, then roslib in a script tag. This makes both files easily cachable and small.

[MatthijsBurgh]

Lets include this in v2.

[MatthijsBurgh]

This will indeed require adding back loading the eventemitter CDN in the examples

[EzraBrooks]

Finally going to get this change in via #688 🙂