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

Distribute the browserified module as part of the NPM package #310

Merged
merged 1 commit into from
Mar 24, 2016

Conversation

sethkinast
Copy link
Contributor

Closes #309

@jeromew
Copy link
Collaborator

jeromew commented Jun 2, 2015

I could be +1 on this but can you tell me why don't you use browserify in your project to rebuild the browserified version ?

@vqvu do you have an idea why we don't publish the browser package ?

@sethkinast
Copy link
Contributor Author

That would be an option; it just feels messy to reach into my node_modules
folder and manipulate stuff there. It also means a postinstall script.

It's not a huge deal if you prefer not to ship it; I just ship a copy in
the vendor folder with the app currently.

On Tue, Jun 2, 2015, 2:45 AM jeromew notifications@github.com wrote:

I could be +1 on this but can you tell me why don't you use browserify in
your project to rebuild the browserified version ?

@vqvu https://github.com/vqvu do you have an idea why we don't publish
the browser package ?


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

@vqvu
Copy link
Collaborator

vqvu commented Jun 2, 2015

@jeromew I don't know. Maybe because npm is usually used for node applications, and the browser package is useless there.

I'm ambivalent about this, but I don't see the harm. Worst case, people download an addition ~180KB.

So I guess that's a weak +1 for me.

@apaleslimghost
Copy link
Collaborator

@sethkinast just to clear something up: you're using npm to install modules to be used in the browser, but you're not using Browserify yourself to build the browser bundle? You're just including script files from node_modules?

If so, that's a very unusual use case for npm, and not one I think we should support.

However, we don't have to publish the bundle to npm: you could install Highland with npm directly from Github, by running npm install --save caolan/highland. The bowserified file is checked in to the repo.

@jgrund
Copy link
Collaborator

jgrund commented Mar 23, 2016

I'm +1 on this as well. It's more common now for people to use a loader like https://github.com/systemjs/systemjs when building a browser app.

Given that, systemjs can load modules in commonJS format, but it's not going work trying to import modules built into the node runtime.

In general it's friendlier to ship the compiled bits, and most projects that are expected to run in browser / server environments do this.

@jgrund
Copy link
Collaborator

jgrund commented Mar 23, 2016

We also ship the browserified bits via bower, but most people have switched from bower to npm for browser development.

@vqvu
Copy link
Collaborator

vqvu commented Mar 24, 2016

Ok. We have a +1, a -1 and two weak +1 (+0.5?). Positive number wins.

I see no obvious downside to this change either, so I'm happy to merge this.

@vqvu vqvu merged commit 958fe54 into caolan:master Mar 24, 2016
@vqvu
Copy link
Collaborator

vqvu commented Mar 24, 2016

I've released 2.7.2 to NPM. That should contain the browserified bundles.

@sethkinast
Copy link
Contributor Author

Well thanks! That will simplify my repo.

On Wed, Mar 23, 2016, 5:26 PM Victor Vu notifications@github.com wrote:

I've released 2.7.2 to NPM. That should contain the browserified bundles.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#310 (comment)

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

Successfully merging this pull request may close these issues.

[request] distribute the browserified highland with the NPM package
5 participants