search

biojs / slush-biojs

DEPRECATED - please use https://github.com/biojs/generator-biojs-webcomponents
https://github.com/biojs/generator-biojs-webcomponents
7 stars 3 forks source link

uses `mkdirp`, not available on 3 machines that I have tried to install biojs libs (build with slush-biojs) on #13

Open doomedramen opened 8 years ago

doomedramen commented 8 years ago

biojs-vcf@1.1.0 build /Users/pagem/Documents/workspace/geefu.io/node_modules/biojs-vcf mkdirp build && browserify -r ./:biojs-vcf -o build/vcf.js

sh: mkdirp: command not found

doomedramen commented 8 years ago

Found the issue, the mkdirp is put into the dev dependencies, even though in production you would still need the lib to be built. I think it would be worth moving that from dev-dependencies to dependencies in the output package.json

manuelcorpas commented 8 years ago

Forwarded this to Dennis

DennisSchwartz commented 8 years ago

Hey Martin. Feel free to issue a pull request if you know how to change this. Otherwise I'm happy to do it myself.

doomedramen commented 8 years ago

Just found out, doing a normal install (with dev deps) does not help ether.

Just passed my project that that a lib (made with biojs-slush) to travis and it cant build ether.

My project: https://github.com/TeamMacLean/geefu.io/blob/master/package.json

which includes: "biojs-vcf": "^1.1.0",

gives the following error on my machines and travis:

biojs-vcf@1.1.0 build /home/travis/build/TeamMacLean/geefu.io/node_modules/biojs-vcf mkdirp build && browserify -r ./:biojs-vcf -o build/vcf.js sh: 1: mkdirp: not found

doomedramen commented 8 years ago

My assumption is that biojs-slush makes mkdirp and browserify a dep of the lib that is generated and that it expects to use both of these from the node_modules/.bin folder but because it is a package on top of this that is being built it does not look in that .bin folder, instead it looks in the one of the top level package. aka, it would build fine if you were JUST building the lib generated with slush-biojs but if you have included said lib in another package.json it does not work as it cannot resolve the .bin folder from that lib

https://travis-ci.org/TeamMacLean/geefu.io/jobs/95320067

doomedramen commented 8 years ago

Just tested this, if I do npm install -g biojs-vcf is works fine, if I include biojs-vcf in a package file and do npm install it complains about mkdirp and browserify not existing. Do you know any other libs built with slush-biojs that I could test?

DennisSchwartz commented 8 years ago

Mhm we're currently in our team meeting. I will take a look at this after. You could try my packages, they are created using slush-biojs if that's what you mean. They are called mplexnet and mplexviz-ngraph (also don't judge my bad coding).

doomedramen commented 8 years ago

ill give them a go now and let you know.

doomedramen commented 8 years ago

yep, same issue with mplexnet, if I install it on its own it is fine (I assume it can see browserify and mkdirp in its .bin folder) but if installed a part of another project it fails.

steps to test:

mkdir test cd test npm init -y npm install mplexnet --save

(make sure browserify and mkdirp are not installed globally before testing or it will give false results) npm rm browserify -g npm rm mkdirp -g

DennisSchwartz commented 8 years ago

We ran into this problem before. One solution is to include browserify and mkdirp in the dependencies. But that will include dev tools in your build and especially browserify is a pretty big package. We didn't come up with a solution yet.

I think what we would need to do is to install the necessary build tools without including them in the actual bundle file. Do you know of any good way to do that?

doomedramen commented 8 years ago

I can only think or dirty, hacky ways, not ideal.

DennisSchwartz commented 8 years ago

I came up with the same conclusion and then forgot about it. I think a clear error message about whats the problem would already help.

How about including a bash script with slush-biojs that will be called in the build command to check if mkdirp and browserify are available and install them if necessary? Do they have to be installed globally to be called from command line?

I'm thinking of something like:

"build": "sh checkDeps.sh && mkdirp build && browserify -r ./:mplexviz-ngraph -o build/bundle.js"
DennisSchwartz commented 8 years ago

Mhm that would of course only work on unix-like environments

doomedramen commented 8 years ago

You could just not force browserify...

DennisSchwartz commented 8 years ago

So give people the choice to use their own bundling system or not use require?

I could add that to the generator.

4ndr01d3 commented 8 years ago

Not completely sure about it, but I think browserify is needed for the registry worker in order to build the snippets because it uses https://wzrd.in/ as a CDN. So I woudn't change that just yet.

On Mon, Dec 7, 2015 at 12:11 PM Dennis Schwartz notifications@github.com wrote:

So give people the choice to use their own bundling system or not use require?

I could add that to the generator.

— Reply to this email directly or view it on GitHub https://github.com/biojs/slush-biojs/issues/13#issuecomment-162507220.

wilzbach commented 8 years ago

guys - have a look here: https://docs.npmjs.com/misc/scripts

I guess your issue is the difference of build and prepublish, I took the liberty to change that:

https://github.com/biojs/slush-biojs/commit/baddb29498ba2da6e1d335c1b0c1a3955dd36a75

You could just not force browserify...

Well you are not the first one who complains ;-) If we manage to get biojs3 in a better state, we could even deprecate this slush tool ;-)

doomedramen commented 8 years ago

So glad I am not the only person that dislikes forcing browserify. It should be 100% optional to the end user for many reasons but the biggest is: my project uses 6 biojs-visualizations, all 6 are pre browserified.... the site loads wayyyy more data downloaded than would be required if I just browserified all 6 into one bundle from the SRC.

wilzbach commented 8 years ago

So glad I am not the only person that dislikes forcing browserify. It should be 100% optional to the end user for many reasons but the biggest is

Yep webpack and other tools are great too ;-) However at the time of biojs2 browserify was the best tool out there & we had to choice one way for the registry. Yep with biojs3 we want to go a different route (I am afraid I am really missing the time to pursue)

https://github.com/biojs/biojs3/issues/4

my project uses 6 biojs-visualizations, all 6 are pre browserified.... the site loads wayyyy more data downloaded than would be required if I just browserified all 6 into one bundle from the SRC.

"Pre browserification" is really something optional, since there are service like wzrd.in that do this automatically -> I am afraid, but you have to run browserify yourself if you want to bundle them ;-)

doomedramen commented 8 years ago

These is also the options to handle dependencies yourself, flat style, I prefer this approach as there are no worries about duplicate imports. Works well with tools like bower.

Githubissues.
  • Githubissues is a development platform for aggregating issues.