Support devDependencies

[tf]

Hello,

I think it would be great if ender supported devDependencies (it doesn't at the moment, rigth?). I'd like to put my testing libraries in there, and have a special ender --dev or whatever include those for my test suite build.

What do you think? I might look into creating a patch if other's also like the idea.

Best
Tim

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/742663-support-devdependencies?utm_campaign=plugin&utm_content=tracker%2F165667&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F165667&utm_medium=issues&utm_source=github).

[ded]

i imagine it being similar to npm install --dev but i'd have to see a full usecase on how one develops using the regular build process vs. dev, and how they co-exist, and why

[tf]

In my projects, I use jasmine a lot. Right now I need to bundle a version of jasmine inside some test/vendor/jasmine directory or add it as a git submodule. Wouldn't it be nice if I could specify jasmine as a devDependency? To obtain the jasmine css file, still some feature as discussed in #44 would be needed. But also for further testing libraries, I think this would be the perfect approach. Maybe ender could just generate a separate ender-dev.js file if you run build with a --dev flag? That could be included in the test runner html, keeping the production code separated from dev dependencies.

[vic]

If [1] gets merged, you could have an ender-dev.json file (that is just like a package.json file with a dependencies hash) and feed it to ender.

[1] #125

[tf]

In view of the 1.0 rewrite, I'd like to bump this issue. It still would be very helpful if ender had a --dev option to include devDependencies from a project's package.json. That way one could generate a build which could be used in jasmine test runner files etc.

Already haven taken a peek into the new ender source, I see two areas which would have to be amended:

1. Internally ender calls npm.commands.install(['.']) which already would install top level devDependencies if not some unfortunate caching of the parsed package.json would cause npm to ignore the passed dev option.
2. If I understand correctly, ender recreates the dependency tree itself. While it should suffice to add devDependencies inside package-util/getDependenciesFromJSON, I'm not sure how to pass a dev option "down there". npm's default behavior to only include devDependencies of the top level package would also be important to follow - we do not want to install devDependencies of required libraries. Still at the moment I have no idea how to distinguish that package in the process.

Ok, I hope some of this makes sense. If someone could give me a few pointers, I'd be glad to help implement this feature.

Best
Tim

[rvagg]

I'm still not entirely clear of the use case, could you explain it with an example perhaps? If you just wanted to include the Jasmine CSS etc. then wouldn't an npm install suffice? Or are you suggesting that it would be good to bypass npm completely and do this all through Ender?

To get --dev passed through, just add it in args-parse.js, there should just be one place you need to add it. Then it gets attached to the object returned from there which is passed around variously as args or options (an inconsistency I need to sort out, I'd like to go with the latter name). You'll see this come in all of the main-x.js modules in the exec() call as the first argument. Here for example. It's usually available most places in the system and if it's not where you need it then just add it as an argument to whatever function you need it in. For a Boolean argument you can just do this kind of thing: if (options.dev).

From what I understand your saying, you just want to npm-install the devDependencies so you could add an extra argument to repository.install() that gives the --dev to npm--except that you mention a caching problem, which I don't understand (but could fully imagine exists). Wouldn't this be the first time npm reads that package.json because we're not doing an install on it until that call?

If you just want the top level devDependencies then perhaps you can just apply some trickery here. You could have main-build-util.js generate two package lists for you, one to send to npm for an install (which would include devDependencies) and one to pass through to handle() which is the bit we care about for building the output file. Or, if you're wanting to include devDependencies in your output build then you could pass the amended package list through there too (although I'm not sure I see this making sense).

So, an exec might look like this:

  , exec = function (options, out, callback) {
      var packages = buildUtil.packageList(options, false)
        , installPackages = options.dev ? buildUtil.packageList(options, true) : packages
        , handler = handle.bind(null, options, packages, out, callback)

      out && out.buildInit(packages)

      packageUtil.preparePackagesDirectory(function (err) {
        if (err) return callback(err) // wrapped in util.js

        repository.setup(function (err) {
          if (err) return callback(err) // wrapped in repository.js
          repository.install(installPackages, handler)
        })
      })
    }

with packageList() doing some fancy work depending on the second argument. I guess it'd check to see if '.' (or cwd) were included in the package list and if so, read ./package.json and pull out devDependencies to add to the list. package-util.js can read and parse the package.json for you (I think a readPackageJSON([], '.', cb) would do it).

The whole internal dependency tree build could probably be avoided if you just want to npm-install the devDependencies and not do anything fancy with them as part of your build. Even then, if you wanted to included them in your output file then you really just need to change buildUtil.packageList() to add them to your package list and I don't think you'd need to do much beyond that.

Does that make sense? Does that sound like what you're wanting to do?

[tf]

Thank you for your detailed answer. I hacked away on a solution this morning and ended up with something quite similar to what you are suggesting. But first of all let me clarify my use case.

We are about to split an existing app into a bunch of npm modules. Each module consists of of some js files which are processed in a custom, rake based build step. The build task generates two main products: a main.js file containing a concatenation of all lib files and a test/unit.js file containing a concatenation of all jasmine specs. When working on a concrete app, I can write a package.json declaring which of these modules I need and use ender to assemble a file which contains all my dependencies. Now I also want to be able to extract testing helpers which are used by multiple modules and package them up as separate npm modules. That way I can version them and enhance them progressively. Those clearly do not belong into the final app, meaning they can not be "normal" dependencies. With the --dev flag, I can extend my build process to generate the following three files for each module:

• a test/unit.js as above
• a test/ender-dev.js containing my module's code, its dependencies and devDependencies
• a test/runner.html which only contains script tags referencing the above two files

Now I am free to require testing helper modules inside my unit tests along with other modules. Furthermore I can easily run the unit tests of each module, while having zero boiler-plate code in each individual repository.

---

Now to my proposed solution. By default npm installs devDependencies of the top level module. So either by further understanding the caching problem I mentioned or by doing a separate repository.install() as you suggest, it should be possible to install those modules to the node_modules folder. If I understand the ender source correctly, constructDependencyTree not only traverses package.json files but also makes a directory listing of the node_modules directory. So in all my tests the dependency tree already included the devDependencies. So all there is left to do is to append the dev modules to the list of packages which is passed to the handle function. This places the dev modules at the end of the ender file, which is the right place in my opinion since no other module should be allowed to depend on those.

I hope I could clarify things a little bit. Later today I will fork the repository and try to implement the feature since I really see no better way of solving our problem.

Thanks again for taking the time to talk this through!

[tf]

I have pushed some commits to the devdep branch of ender fork. I still have to finish the integration tests, for which I started to add fixture file support for functionalCommon.runEnder. That way we could also get test coverage for normal installation from package.json files.

[rvagg]

Nice work, still trying to get my head totally around the use-case and your implementation, I think I just need a bit more time to be able to sit down with this and consider how it fits into the broader picture--i.e. if we're extend ./package.json usage for the special ender build ./ case then how can this be made to fit in well with that.

Some questions for you:

The devDependencies are only appended to forBuild, won't you need them to be npm installed too? tf/Ender@d0f1372...401ebc9#L1R80

It's interesting that you switched around install(['.']) with install(withoutCwd), was there a specific reason for doing this? Does that help out in the npm caching issue and get npm to install devDependencies for you without asking for them by name? tf/Ender@d0f1372...401ebc9#L4R156

Good work on adding some unit tests for this, the quirks of npm also point to the need for good functional tests for it too so we know when (not if) it breaks upstream.

I'll see if I can set aside some time this week to think about this in more detail.

[rvagg]

(updated that last comment with better links)

[tf]

I found some more time to work on this. First of all I've pushed an enhanced version of the runEnder test helper to the filefixtures branch of my repository. Basically runEnder now takes a hash with fixture file names as keys and file contents as values and places these files in the test sandbox directory before running the test. As an example, I've added a functional test for building from a project's package.json. If you want to, I can make a pull request of it since it functional tests already present ender features.

Furthermore, I rebased my devdeps branch against the filefixtures branch and added a functional test for building from a package.json with the new --dev option.

I've started to use specific versions of packages installed in functional tests, which I think would be a good idea for all other functional test, too. Otherwise a package update which changes dependencies could break ender functional tests.

---

UPDATE: Please note that much of the following discussion in this post is made obsolete by the simpler implementation I describe in the next posts. So you might just skim through this.

Now to your questions: By default npm installs devDependencies from ./package.json if you invoke npm install .. Explicitly passing a --dev option would install devDependencies from all packages in the dependency tree, which surely is not what we want. I had to switch around the install commands since npm only parses and caches the package.json during the first install. If we start with the withoutCwd case, devDependencies are omitted during this parsing and when npm implicitly adds the --dev option for the first level of the install(['.']) command, nothing happens because the cached package.json version does not include devDependencies.

The only case which still has to be handled is when the npm config option production is set to true. In this case devDependencies are always omitted - also in the top level package. Even though passing --dev in a production environment is a rather pathological case, we still should try to incorporate a warning.

I realize that most of the above explanation should be added as a comment somewhere in the source code. It's a bit sad that the rather straightforward main-build.js has to be obfuscated with concepts like packages.forBuild and packages.forInstall. But at the moment, I do not see a much better way to get this functionality.

[tf]

The more I think about it, the division into packages.toBuild and packages.toInstall might not even be necessary. It should not hurt to pass the list of devDependencies to npm intall explicitly. Given the current behavior of npm, it might be redundant, but even more it should fix those production config option headaches I was talking about. Will give that a try.

[tf]

This idea worked out fine. The devdep2 branch contains the amended commits together with some more functional tests. From my point of view, the final question is how to display dev dependencies in the info command now. I yet have to fully understand, how ender goes about parsing it's output files to reconstruct this information. I might look into this issue once other's have confirmed that the --dev flag has a future in the upstream repository.

@rvagg If you need more details on my particular use case, I'd be happy to answer.

[tf]

And as another note: Reversing the order of the npm installs is now also no longer necessary since we no longer rely on npm's handling of devDependencies (which probably is a good thing).

[rvagg]

I like what you've done with the code, nice and clean, but yes I'd really like more detail on your use-case to understand this a bit better. I'm not a Jasmine user so perhaps I need a bit of background there to understand what kind of code you need to go in to your build to support your tests.

Personally I'd prefer unit tests to not require anything extra of my production code in order to have them run and I don't mind exposing more than the plain API in order to achieve this (one of the reasons that this CLI code is in so many modules, much more exposed for unit tests to access). But I suspect I might be missing the point of what you're trying to achieve. Perhaps if you could imagine having to write this up for a wiki page (which we'd probably need for this feature), why would people want to use --dev?

[tf]

I really do not think that this has much to do with jasmine. Much rather - with testing frameworks getting a lot more modular - I'd imagine declaring your favorite testing library, mocking library and expectations library as devDependencies. Additionally we have a lot of custom assertions and syntactic sugar helpers for testing things like promises and asynchronous constructs. I'd like to package, version and share those between projects. Does this make sense to you?

Earlier you talked about ender specific extensions of the package.json file which the --dev option would have to fit in with. Could you elaborate on this?

[rvagg]

OK, I get it now, sorry to be slow. You're wanting to include the testing & related libs in there, Jasmine, Sinon, custom assertions, etc. So you'd end up with an ender-dev.js that is the same as an ender.js would be but with the additional stuff on the end.

#131 is where I've tried to pull together some of the thinking about enhancing package.json, in particular see point 3 under Proposals. Building CWD (./) is increasingly becoming a special case where you want to (and can) do things that you can do with a build that just involves modules from npm or non-CWD paths.

Your extension fits nicely into this of course because you're wanting to only have this executed when you include CWD in a build. The idea in proposal 3 is that package.json becomes a substitute for an ender.json config file (perhaps we could have both), where you can put things into the "ender" key that would substitute for supplying command line arguments. My concern is simply making sure that your CWD-special-case fits in nicely with the proposed CWD-special-case use.

Having said all that though I'm not sure I can see a problem with this, plus you have the early-implementer advantage. PackageDescriptor.js is the beginning of implementation of #131, we'd just add "devDependencies" as something that can be overridden with the "ender" key (which would be handy if you're using npm install separate from ender build on CWD.

Feel free to put in a PR any time and we can use it to discuss before it gets committed.

Some additional thoughts for now:

• ender build is supposed to work the same as ender build ., this may or may not be something you want to take into account with your tests; I'll leave that up to you.
• I'm wondering if the default output filename should be adjusted for the --dev case, perhaps ender-dev.js & ender-dev.min.js along the lines of what you suggested above, mainly as an indicator that the build wasn't intended for production, but it can still be overridden with --output of course. What are your thoughts on this? Trivial to do in util.js
• I wouldn't mind seeing test(s), either unit or functional, for the use of --dev with a non-CWD build (on module(s) with "devDependencies"), to both show that it doesn't do anything and also to document for future reference that it's not supposed to do anything. Perhaps in future we can have errors printed out when you use command line arguments that aren't going to do anything on your current command.

[tf]

I agree with all three suggestions. I will see when I have some time to make these additions. In parallel I will start using my ender fork locally. Maybe still other needs come up, even though I do not reckon so. I'm still unclear on how to amend the info command though. I'd like to avoid duplicating the devDependencies logic for displaying the correct dev packages when parseContext sees a --dev option in the stored ender command.

[rvagg]

here's an idea, make packageList() give an object instead of an array where the keys are your packages and the values are a purpose descriptor, so you'd get something like:

{ "ender-js": "client", "bonzo": "*", "bean: "*", "sinon": "dev", "foobar:" "dev" }

Use Object.keys() where you just need the array (relying on key ordering should be fine, I think the only hiccup is where a key is a Number in V8). Then when you call generateAndPrint() in main-info.js you can pass this object in and do something special with the dev dependencies in the output ("dev" label, different colour, whatever). For the plain ender info case, after parseContext() you could pass the returned context back through packageList() to get the same object (mainBuildUtil.packageList(context.options, cb) I guess).

Make sense?

[rvagg]

I'm really sorry @tf but I've done a ton of work in the build/install/npm area and messed up your changes on the latest branch. main-build-util.js has been split up into DependencyTree.js (now an object with instance methods), install.js and install-util.js but the good news is that we've taken a lot of control back from npm so repository.js has been simplified somewhat (install() now just installs, install.js decides what to install and in what order).
Best diff view is probably here. Let me know if you need any help or hints, I'd be happy to accept a PR on devDependencies when you get it working on the latest.
Cheers.

[tf]

Never mind. I've been meaning to finish this a long time ago. I'll try to look into this as soon as possible.