search

palavatv / palava

palava browser-to-browser video
https://palava.tv
30 stars 3 forks source link

CommonJS/npm/browserify #7

Closed thammi closed 10 years ago

thammi commented 10 years ago

Issue by thammi Thursday Sep 04, 2014 at 20:02 GMT Originally opened as https://github.com/palavatv/palava-client/pull/10

Makes the library loadable as CommonJS module/package. This adds support for npm/browserify and their require() functions.

I had to add palava = @palava to the top of most files and $ = @$ to the top of most files because we were depending on this being the global namespace (as a result of the way sprockets implements #= require).

Additionally there is now module.coffee which tries to load JQuery and EventEmitter with require() if they are not in the global namespace and exports the palava namespace.

thammi included the following code: https://github.com/palavatv/palava-client/pull/10/commits

thammi commented 10 years ago

Comment by janlelis Wednesday Oct 08, 2014 at 13:36 GMT

thank you! looks good to me. some related things i've noticed:

thammi commented 10 years ago

Comment by thammi Saturday Oct 11, 2014 at 15:57 GMT

Bundling the dependencies without a mechanism like require()/AMD/... would pollute the namespace of the user and might cause version conflicts if the user manually includes the libraries.

thammi commented 10 years ago

Comment by janlelis Saturday Oct 11, 2014 at 21:05 GMT

@thammi I agree. What I have in mind is providing an extra version (and don't bundle deps into "palava.js"