npm modules are loaded from wrong path

[jamesshore]

As @abbakym mentioned in #16:

karma-commonjs tries to load dependencies by their immediate names, e.g. root/lodash.js and not root/lodash/lodash.js.

This makes npm modules pretty much impossible to use with karma-commonjs.

[pkozlowski-opensource]

Hmm, but this would require us to parse package.json and it would be the same as #16. We could extend the current behaviour to better match http://nodejs.org/api/modules.html#modules_file_modules by trying out different matches (index.js, index.json etc.). In this sense require('root/lodash') could try to load root/lodash/index.js.

But to load root/lodash/lodash.js we need to be aware of the package.json and support this part of the spec: http://nodejs.org/api/modules.html#modules_folders_as_modules

In short: I don't think we can resolve require('root/lodash') to root/lodash/lodash.js without reading package.json and for this we've got an issue opened already.

[jamesshore]

Yes, I think you're right. We could work around this issue by looking for require("package") in root/package/index.js by default, but we really need to follow the entire spec, including parsing package.json

However, the current behavior is for require("package") to look for root/package.js, correct? Is there any case where that's the right thing to do?

Perhaps we should disable support for node_modules entirely until we get a proper spec-compliant implementation working.

[pkozlowski-opensource]

@jamesshore there are use-cases for the current behaviour, it is part of the spec and I'm using it in my projects (I think some other Karma tests depend on this as well). The use-case is very simple - you want to test your project written CommonJS-style where there is a bunch of files. So we definitively shouldn't disable this.

The problem with require("foo/bar") is that we don't know if bar is a file or a folder (module) with index.js / file specified in package.json and we need to try-out all the possibilities.

For now what we can easily make sure that we test "foo/bar/index.js" in response to require("foo/bar").
Then we need to either learn how to parse package.json or intoduce some kind of a map.

The current behaviour isn't buggy, it is incomplete. But even in this incomplete form is still very useful so IMO we shouldn't be taking anything out, we should just be adding things toward full node.js module loading spec, if this is makes sense in a browser context of course.

[jamesshore]

@pkozlowski-opensource I'll defer to you on this one. I don't use npm modules with my client-side code, so this doesn't come up for me. Sorry for my ignorance. :-)

[pkozlowski-opensource]

@jamesshore I'm in the exactly the same situation, I'm using CJS module for browser code but not to load npm modules as-they-are. I mean, I'm loading my own code written CJS-style but not existing npm ones.

I think we can improve current support for the node module loading spec, it is just that this is not my use-case so wasn't digging into this too much. Certain things are easy to do but other are more involved (parsing all the package.json for example).

[pkozlowski-opensource]

Going to close this one as it is exactly the same issue as in #16 . Looking at the lodash repository we can see that the built version is stored in the dist folder: https://github.com/lodash/lodash/tree/master/dist and this exactly where the package.json is pointing to: https://github.com/lodash/lodash/blob/master/package.json#L7

The root/lodash/lodash.js might look that something that should be resolved too from require('lodash') but this is purely by accident as in this particular case the npm package name (and GH repo name) matches the main file name. But this is just one specific case and not something general.

[givankin]

Going to close this one as it is exactly the same issue as in #17

Isn't it the #17? You probably meant that it is the same as #16 or wanted to close the #16 )

[pkozlowski-opensource]

@abbakym you are right, I've meant to close this one as duplicate of #16, thnx for the heads up!