grommet / grommet-toolbox

[DEPRECATED] Developer Environment for Grommet applications
Apache License 2.0
13 stars 24 forks source link

Added missing dependency. #9

Closed coltonw closed 8 years ago

coltonw commented 8 years ago

I am not confident in my npm-fu, but it seems like since you are directly using yargs in this repo, it should be a direct dependency. Currently it is an indirect dependency because some of your dependencies have it as a dependency but it seems fragile to rely on that and possibly broken in some cases.

The reason this came up is that when I was trying to automate my grommet ui build, it failed with the following error:

module.js:327
    throw err;
    ^

Error: Cannot find module 'yargs'
    at Function.Module._resolveFilename (module.js:325:15)
    at Function.Module._load (module.js:276:25)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (/opt/mount1/home/jenkins/workspace/SFRM-Demo-develop/Grommet-UI-Demo/node_modules/grommet-toolbox/lib/gulp-tasks-dev.js:12:14)
    at Module._compile (module.js:409:26)
    at Module._extensions..js (module.js:416:10)
    at Object.require.extensions.(anonymous function) [as .js] (/opt/mount1/home/jenkins/workspace/SFRM-Demo-develop/Grommet-UI-Demo/node_modules/babel-core/node_modules/babel-register/lib/node.js:166:7)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
Build step 'Execute shell' marked build as failure
Archiving artifacts
Finished: FAILURE

I cannot reproduce the error locally and so am not sure how to test this fix, but thought this might be goodness anyways.

Sorry for the super long-winded pull request description for a one line change!

alansouzati commented 8 years ago

makes fully sense, sorry for missing that. if it is a direct dependency it should be inside package.json.

Starting with NPM 3 all dependencies are flat, I'm guessing in your CI you have NPM 2 and locally you have NPM 3.

coltonw commented 8 years ago

That was exactly the issue. I figured it out a bit later but thanks for taking the change. If figured it was just goodness.