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

CommonJS/npm/browserify #7

Closed
thammi opened this issue Nov 23, 2014 · 3 comments
Closed

CommonJS/npm/browserify #7

thammi opened this issue Nov 23, 2014 · 3 comments

Comments

@thammi
Copy link
Member

thammi commented Nov 23, 2014

Issue by thammi
Thursday Sep 04, 2014 at 20:02 GMT
Originally opened as palavatv/palava-client#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
Copy link
Member Author

thammi commented Nov 23, 2014

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


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

  • bower.json is missing jquery
  • we should provide a bundle with all dependencies... should this even be in the .min. ("production") version so you can just drop it into your code and it works?

@thammi
Copy link
Member Author

thammi commented Nov 23, 2014

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
Copy link
Member Author

thammi commented Nov 23, 2014

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"

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

No branches or pull requests

2 participants