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

publish dist on npm #265

Closed
thepian opened this issue Oct 3, 2015 · 24 comments
Closed

publish dist on npm #265

thepian opened this issue Oct 3, 2015 · 24 comments

Comments

@thepian
Copy link

thepian commented Oct 3, 2015

You are excluding dist on .npmignore

For SocketStream we want to include the client to serve the source file rather than copy it. This would make it much easier

@brycekahle
Copy link
Contributor

Can you not browserify it?

@thepian
Copy link
Author

thepian commented Oct 30, 2015

Not really, I guess I will have to continue to bake it into SocketStream. It means that users will be on old client code. But since they seem to have been fine with 3 years old code so far I guess it is Ok.

I guess it will only be a year or two before WS compatibility libraries can be dropped.

@brycekahle
Copy link
Contributor

Because this module also works from node, it does not make sense to ship an additional copy of the library for those users. This module can be bundled with browserify (and possibly webpack but I haven't tried), so if you want to use the npm module, I suggest you give that a try.

@thepian
Copy link
Author

thepian commented Nov 1, 2015

I find it baffling that you essentially require browser uses to use Browserify, but I'm sure you know the direction you want to go with the Library. For SocketStream we don't have that option. The refactoring would be weeks of work.

My solution is to bundle the latest client and leave future major releases unsupported out-of-the-box. I think that should be fine for at least a year or two.

@thepian thepian closed this as completed Nov 1, 2015
@brycekahle
Copy link
Contributor

Browser users can use the CDN hosted versions at cdnjs and jsdelivr, or simply download and host the library themselves from the dist folder. This is built in UMD, so you can require this file as well.

Node users can install the npm module.

Browser users who also want to use npm, can install the node module but are responsible for running browserify or another bundler intended for web usage. It sounds like you want npm to be bower. It is not.

@AlexanderTserkovniy
Copy link

+1 if you want to have the library in npm, than it should contain dist with compiled version.
Please add dist, it is common practice to have such a folder if you using some front-end solution.

@AlexanderTserkovniy
Copy link

And look, I made browserify in your folder on file entry.js and instead of working code I have got error Uncaught ReferenceError: SockJS is not defined.
At least please give us cli command how to compile your library properly.

@AlexanderTserkovniy
Copy link

Ok, so to bundle via browserify the client library you need to run this code in command line within lib folder:

browserify entry.js --standalone SockJS > bundle.js

P.S. also this might help: http://stackoverflow.com/questions/15590702/how-to-get-minified-output-with-browserify

@brycekahle
Copy link
Contributor

The easiest way would be to use gulp and run npm run gulp stable-release which will output the file into dist.

@AlexanderTserkovniy
Copy link

Thanks a lot!

@enriquecaballero
Copy link

You do know there is a pre-install hook that can be used to fire gulp on npm install?

@enriquecaballero
Copy link

It is bad practice to touch or edit whatever files are inside the node_modules directory...

@markgeraty
Copy link

+1 for running the gulp stable-release command you posted above in as a pre-install hook

@kirankashalkar
Copy link

Looks like even the gulpfile is in .npmignore.
This means one cannot use the package directly from npm to build, even with git+https option.
The source has to be checked out from github and then built.
That's not a viable production option for many.
Also, reading from a CDN is not always possible too as firewalls may prevent that.

@jettisonjoe
Copy link

+1 to what @kirankashalkar just said. Firewalls are my whole reason for wanting to use sockjs instead of straight websockets, and the same firewall prevents my clients from hitting the CDN. Every other dep for my app includes a prebuilt script for the browser in their npm package. sockjs-client not only neglects to provide a built script, but also disables building from the npm package by excluding the gulpfile. Why even have an npm package?

@mborman
Copy link

mborman commented Aug 11, 2016

Old... but I agree that having the dist folder in npm is a nice-to-have. It is not a necessity though...

In the absence of browserify I frequently use npm scripts to handle project lifecycle / builds. See https://www.keithcirkel.co.uk/how-to-use-npm-as-a-build-tool. It is trivial to write an npm script that copies a file from a dist folder to wherever it needs to go for a browser-based project. It is substantially more complicated to check out source and run the build task prior to copying the dist artifact. Rather than create a fragile npm script I host the unminified version locally (It gets minified as part of the build). Only downside is updates to SockJs are now a separate manual process. Not a deal breaker.

Another option is to create a package that contains only the built file and publish it to a private registry. I've done this in the past using sinopia.

@andrewfinnell
Copy link

I do NOT see the UMD version in the npm bundle. In essence if one is using a AMD loader the npm module is unusable.

Bower is dead. NPM had become a defacto for both client and server side. It's easy enough to link node module directories into a /lib dir for Web Apps. CDN isn't acceptable for AMD users nor users who wish to not hit other domains for internal apps.

It seems too clever to not include the dist directory. The gain out weighs any possible negative of the duplicate files. At the very least have the npm module build the UMD version when installed.

This and Kendo are the only two packages who have committed to such convoluted npm packages for the sake of "being right."

For anyone having issues use github:parlew/sockjs-client or fork the repo, fix the .npmignore then use the npm+git integration. It seems so silly to enforce restrictions where none are needed.

Some projects can't use browserify or webpack. It's not on purpose it's just life.

@TomFoyster
Copy link

Trying to use this via Aurelia and JSPM. I installed the package, had the issues with the dist/ folder, came here, and uninstalled the package.
Dozens of other packages "just work" - why must this one be different? I'm not going to run bower or gulp within my jspm_packages/ directory.

It's a shame, as it's clear to see the benefits of SockJS over native WebSocket's or even Socket.io - but if it doesn't fit in our workflow (a workflow based on how dozens of other packages are managed) we can't use it.

@andrewfinnell
Copy link

andrewfinnell commented Oct 21, 2016

@TomFoyster I've already fixed it. It doesn't require you to do anything except change your package.json file to this:

"dependencies" : {
  "sockjs-client": "github:parlew/sockjs-client"
}

This will pull down the latest version with the directory you need in it. We can submit a pull request to the main repo and make our case. Until then, it's easy enough to pull the module as stated above. And it doesn't require anything extra.

FYI: The repo sockjs/sockjs-client is not my repo nor have I committed anything to it. I just forked it into my OSS public companies repo and made the changes there. Then rely on the npm+git features to pull it. I'll obtain a NPMJS account soon enough, then this problem will become moot.

@JeremyCarlsten
Copy link

+1 It's pretty much standard practice now to include the dist folder. I don't want to have some special build process for one odd package just because the authors refuse to include a dist folder.
@afinnell Thanks, we will use that version for now.

@vahidshirvani
Copy link

We use spring-boot as our backend and not node. Node binary is only used for some typescript compilation. I would highly appreciate if you could include the dist/ folder.

@brycekahle
Copy link
Contributor

1.1.2 on npm contains a full dist folder containing minified and non-minified versions, along with sourcemaps for both.

@JeremyCarlsten
Copy link

Thank you sir!

@jimaek
Copy link

jimaek commented May 8, 2017

I wanted to let you know that the new backend is live. It pulls files directly from npm and github https://github.com/jsdelivr/jsdelivr#usage

Example https://cdn.jsdelivr.net/npm/jquery@3.1.1
Example directory listing https://cdn.jsdelivr.net/npm/jquery@3.1.1/

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