componentjs / component

frontend package manager and build tool for modular web applications
https://github.com/componentjs/guide
MIT License
4.55k stars 306 forks source link

component-install --dev installs components to "dependencies" instead of "development" #566

Closed jasonkuhrt closed 9 years ago

jasonkuhrt commented 10 years ago

Starting with this:

c install -h                                                                                                                              

  Usage: component-install [name ...]

  Options:

    -h, --help   output usage information
    -d, --dev    install development dependencies
    -p, --proxy  use a proxy, eg. http://localhost:3128
{
  "name": "oauth2-implicit",
  "veresion": "0.0.0",
  "main": "index.js",
  "scripts": [
    "index.js",
    "lib/**/*.js"
  ],
  "dependencies": {
    "yields/store": "~0.2.0",
  }
}

Then:

⧑ DEBUG=* c install chaijs/chai --dev                                                                                                       
  remotes:local checking local components at /Users/jasonkuhrt/projects/oauth2-implicit/components +0ms
  component-resolver remote not set - defaulting to remotes's defaults +0ms
  component-resolver:locals resolving local at "/Users/jasonkuhrt/projects/oauth2-implicit" +0ms
  component-resolver resolving "root" +3ms
  component-resolver remaining dependencies: 1 +3ms
  component-resolver remaining semver: 0 +0ms
  component-resolver finished resolving locals +1ms
  component-resolver finished resolving dependencies (1) +0ms
  component-resolver:semver resolving semver chaijs/chai@* +0ms
  remotes:local resolving local remote +12ms
  remotes:local checking folder: /Users/jasonkuhrt/projects/oauth2-implicit/components/chaijs/chai +1ms
  remotes:local got folders: 1.9.1 +0ms
  remotes:local checking folder: /Users/jasonkuhrt/projects/oauth2-implicit/components/chaijs/chai +1ms
  remotes:local got folders: 1.9.1 +0ms
  component-resolver:dependencies resolving dependency chaijs/chai@1.9.1 +0ms
  component-resolver:dependencies searching ["local","github","bitbucket"] for chaijs/chai@1.9.1 +0ms
  remotes:local resolving local remote +2ms
  remotes:local checking folder: /Users/jasonkuhrt/projects/oauth2-implicit/components/chaijs/chai +0ms
  remotes:local got folders: 1.9.1 +1ms
  component-resolver:dependencies found chaijs/chai@1.9.1 from remote "local" +3ms
  component-resolver resolving "chaijs/chai" +8ms
  component-resolver:dependencies resolving dependency chaijs/assertion-error@1.0.0 +0ms
  component-resolver:dependencies searching ["local","github","bitbucket"] for chaijs/assertion-error@1.0.0 +0ms
  remotes:local resolving local remote +2ms
  remotes:local checking folder: /Users/jasonkuhrt/projects/oauth2-implicit/components/chaijs/assertion-error +0ms
  component-resolver:dependencies resolving dependency chaijs/deep-eql@0.1.3 +0ms
  component-resolver:dependencies searching ["local","github","bitbucket"] for chaijs/deep-eql@0.1.3 +0ms
  remotes:local resolving local remote +1ms
  remotes:local checking folder: /Users/jasonkuhrt/projects/oauth2-implicit/components/chaijs/deep-eql +0ms
  component-resolver remaining dependencies: 2 +1ms
  component-resolver remaining semver: 1 +0ms
  component-resolver:semver resolved semver chaijs/chai@* -> chaijs/chai@1.9.1 +8ms
  component-resolver finished resolving semver +1ms
  remotes:local got folders: 1.0.0 +1ms
  remotes:local got folders: 0.1.3 +0ms
  component-downloader "/Users/jasonkuhrt/projects/oauth2-implicit/components/chaijs/chai/1.9.1" exists, skipping downloading. +0ms
  component-resolver:dependencies found chaijs/assertion-error@1.0.0 from remote "local" +2ms
  component-resolver resolving "chaijs/assertion-error" +1ms
  component-resolver remaining dependencies: 2 +0ms
  component-resolver remaining semver: 0 +0ms
  component-resolver:dependencies resolved dependencies of "chaijs/chai" +1ms
  component-resolver:dependencies found chaijs/deep-eql@0.1.3 from remote "local" +0ms
  component-resolver resolving "chaijs/deep-eql" +0ms
  component-resolver:dependencies resolving dependency chaijs/type-detect@0.1.1 +0ms
  component-resolver:dependencies searching ["local","github","bitbucket"] for chaijs/type-detect@0.1.1 +0ms
  remotes:local resolving local remote +1ms
  remotes:local checking folder: /Users/jasonkuhrt/projects/oauth2-implicit/components/chaijs/type-detect +0ms
  component-resolver remaining dependencies: 3 +1ms
  component-resolver remaining semver: 0 +0ms
  component-resolver:dependencies resolved dependencies of "chaijs/chai" +1ms
  component-downloader "/Users/jasonkuhrt/projects/oauth2-implicit/components/chaijs/assertion-error/1.0.0" exists, skipping downloading. +2ms
  remotes:local got folders: 0.1.1 +1ms
  component-downloader "/Users/jasonkuhrt/projects/oauth2-implicit/components/chaijs/deep-eql/0.1.3" exists, skipping downloading. +0ms
  component-resolver:dependencies found chaijs/type-detect@0.1.1 from remote "local" +0ms
  component-resolver resolving "chaijs/type-detect" +1ms
  component-resolver remaining dependencies: 1 +0ms
  component-resolver remaining semver: 0 +0ms
  component-resolver:dependencies resolved dependencies of "chaijs/deep-eql" +1ms
  component-resolver finished resolving dependencies(2) +0ms
  component-downloader "/Users/jasonkuhrt/projects/oauth2-implicit/components/chaijs/type-detect/0.1.1" exists, skipping downloading. +1ms
  component-resolver finished installing dependencies +0ms
     install : complete

Results in:

{
  "name": "oauth2-implicit",
  "veresion": "0.0.0",
  "main": "index.js",
  "scripts": [
    "index.js",
    "lib/**/*.js"
  ],
  "dependencies": {
    "yields/store": "~0.2.0",
    "chaijs/chai": "^1.9.1"
  }
}

: (

jasonkuhrt commented 10 years ago

@jonathanong @visionmedia I'm digging in to fix this as my first pr but first must clarify the following.

It appears that --dev is passed through to resolve() via options which in turn installs the development: ... components-of-sub-components. Test coverage seems to confirm this.

Given the above, I need to clarify if this is bug or bad design or good design (if so how so?).

Before "fixing" I want to understand the background problem space in case there is something I am neglecting, I am currently thinking of --dev like npm's --save-dev.

jonathanong commented 10 years ago

this about updating the component.json file? if so, it's probably just oversight and a bug.

it shouldn't install the development dependencies of dependencies. if so, that's a bug. i'm pretty sure i have a test for that.

jasonkuhrt commented 10 years ago

this about updating the component.json file? if so, it's probably just oversight and a bug.

Yes. Ok thanks for confirming.

it shouldn't install the development dependencies of dependencies. if so, that's a bug. i'm pretty sure i have a test for that.

I am / we are confused, this test code:

https://github.com/component/component/blob/master/test/install.js#L128-L140

  it('should install dev deps when --dev is used', function(done){
    exec('bin/component install -d', function(err, stdout){
      if (err) return done(err);
      stdout.should.include('install');
      stdout.should.include('complete');
      var json = require(path.resolve('components/component/tip/1.0.0/component.json'));
      json.name.should.equal('tip');
      var json = require(path.resolve('components/component/popover/1.1.0/component.json'));
      json.name.should.equal('popover');
      assert(exists('components/component/assert/0.3.0'), 'dev deps should be installed');
      done();
    })
  })

Is ensuring that assert is installed due to dependencies depending on assert in their development field. Clear? I couldn't decide if this is odd or not yet, first I am working on the issue that --dev installs to your dependencies field instead of development.

timaschew commented 9 years ago

@jasonkuhrt

I am currently thinking of --dev like npm's --save-dev.

I think components --dev flag is more than the opposite of npm install --production With that assumption the test case (https://github.com/component/component/blob/master/test/install.js#L128-L140) make sense.