I think it would be easier to use if it didn't bundle all of its dependencies.

Can you clarify this statement? What, in particular, would be easier about it.

I'm not opposed to using npm dependencies but the original use case for this library was in the browser, and I'd like to continue to support that as well.

The event emitter library we're using is this one,
https://github.com/Wolfy87/EventEmitter/
https://www.npmjs.com/package/wolfy87-eventemitter

It's unclear which upstream version that crypto-js package is tracking. It'd be nice if an official version was published, https://code.google.com/p/crypto-js/issues/detail?id=44

The bigint library is even more problematic. We've patched a bunch of bugs which are present in the one you've linked.

@arlolra thanks for replying. I'm actually trying to use this on the server side rather than in the browser, and also within a react-native app. But I understand the importance of browserifiability

By easier, I meant a few things:

it would be smaller - for example right now I end up with react-native's own eventemitter, the node events module, and otr's. If I use another lib that depends on crypto-js, I'll end up with two crypto-js modules in my build.

it would allow for more people to help evolve it, e.g. if DSA was a separate module, someone interested in using DSA in their project wouldn't have to drag the whole OTR lib with it. Right now they do, and this is a pretty big downside.

re: bigint, what about https://www.npmjs.org/package/bn.js ? https://github.com/indutny makes very solid stuff

it would be smaller - for example right now I end up with react-native's own eventemitter, the node events module, and otr's. If I use another lib that depends on crypto-js, I'll end up with two crypto-js modules in my build.

That's a good point. An easy win on this front would be to improve the .npmignore file, or include a files section in the package.json. I'm sure we're not trying hard there yet.

It would also be good to check the API compatibility between the event emitter we're including and node's. I suspect that dependency can be removed entirely.

it would allow for more people to help evolve it, e.g. if DSA was a separate module, someone interested in using DSA in their project wouldn't have to drag the whole OTR lib with it. Right now they do, and this is a pretty big downside.

As long as the build script is able to stitch it all together to produce the same bundle, I don't have an issue with splitting it out. However, I believe DSA and OTR are using some shared helpers.

re: bigint, what about https://www.npmjs.org/package/bn.js ? https://github.com/indutny makes very solid stuff

Agreed that indutny is a star. But from the looks of the benchmarks, they're only trying reductions with the relatively smaller primes used in ECC. I wonder how it'd perform with the 1024 and 1536-bit primes OTR uses. There has been some effort spent optimizing the current bigint library's powmod and it still borders on intolerably slow in the browser. Regardless though, it would be nice to switch to a library that's being audited and maintained.

events were pretty easy to mod https://github.com/mvayngrib/otr/tree/events

i'll play around with using browserify https://github.com/mvayngrib/otr/tree/browserify , so far seems to work fine, though I haven't done any perf tests

unfortunately i'm seeing huge diffs, mostly because i changed stuff to CommonJS format, and then nuked the build/dep folder. I may factor those steps out into separate commits at some point. Stay tuned

I'm unlikely to merge a large diff like that; it's too hard to follow. A nice piece to start with would be the s/trigger/emit/ change. The api docs list trigger as an alias so it's safe and matches node. Also, bumping to the latest ee version would be good. Then we can follow up with just replacing the require statements.

Also, see what I said above about .npmignore / files section of package json.

I have mixed feelings about browserify.

yea, totally understand. If you decide to check the actual non-whitespace
changes, you can append ?w=1 to /compare/... urls

i'll try to split it up into a more granular series of commits

edit: i'll think about the .npmignore / files suggestion. trigger/emit was basically what i changed in the events branch

On Tue, May 5, 2015 at 1:35 PM, Arlo Breault [email protected]
wrote:

I'm unlikely to merge a large diff like that; it's too hard to follow. A
nice piece to start with would be the s/trigger/emit/ change. The api docs
https://github.com/Wolfy87/EventEmitter/blob/master/docs/api.md#trigger
list trigger as an alias so it's safe and matches node. Also, bumping to
the latest ee version would be good. Then when can follow up with just
replacing the require statements.

Also, see what I said above about .npmignore / files section of package
json.

I have mixed feelings about browserify.

—
Reply to this email directly or view it on GitHub
#63 (comment).

This issue is more as two years old, but I am also interested in splitting those libraries. My suggestion to still use this library in the browser is a prepublish hook similar to the one in many strophejs plugins. If you like the idea I am happy to start working on this.

rollup seems like a nice tool to let you use ES6 modules, today. Is the proposal to rewrite the library with ES6 module syntax, and then use a build step to compile to CommonJS or AMD formats, as desired? One concern is source mapping.

Is the proposal to rewrite the library with ES6 module syntax, and then use a build step to compile to CommonJS or AMD formats, as desired?

Something like that. Currently I am not sure if we have to compile to CommonJS and AMD, or if I we can have a combined file similar to the current state, but I would try to figure it out and open a pr.

One concern is source mapping

I think this is no big deal, because we probably only have to remove the "module" wrapper and these changes should be ignored by git diff -w.