-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
package.json is not used to resolve module path #16
Comments
@abbakym you are right that the plugin doesn't read package.json at the moment. The idea of a map was discussed before and it kind of makes sense, it is just that none of maintainers had a immediate need for resolution from multiple folders. If you've got a use-case and fancy sending a PR I could help a bit as I might have (in a distant future...) a use case for resolution from multiple root folders. |
I've updated the title. I agree that this is something we should support. I'd like to find a way to do it without requiring users to manually duplicate the package.json "main" entry, although I'm not sure there's an easy way to do so. I think we'd have to read the entire |
@jamesshore I think that @abbakym it in fact raising 2 issues here:
@abbakym if you need both perhaps it would be easier to have it as a separate issue. |
@pkozlowski-opensource Hmm, you might be right. That's not how I read it. @abbakym, if you want to change the |
@jamesshore, @pkozlowski-opensource, thanks for your feedback. Regarding multiple root folders I am in doubt. I am a very recent convert from AMD and what I see in browserify (and, more broadly, in CommonJS front-end) world is that there is much more confusion than there is on a surface ("love browserify and stop worrying" hype). You have bower, components, globals, AMD--and then debowerify, decomponentify, deblobalify etc. etc. While this is the state of affairs which is unlikely to change dramatically in the near future, it would be nice to have a suggested workflow that could cover as much of this as possible. E.g. if I saw in karma-commonjs README: "keep all you dependencies in node_modules, and then we will take good care of them", and this would "just work", I would be happy. |
BTW, besides not parsing What I ended up doing is setting up a build task that copies all the Maybe I am missing something. Can anyone suggest a happy workflow that works for you and allows both to "compile" the code quickly and to run unit tests on (uncompiled) modules? Thanks very much in advance. |
That's related but slightly different. I'll open an issue for it. |
@pkozlowski-opensource, can you help me understand this issue better? Please correct any misunderstandings below:
Is this correct so far? |
Also,
|
@jamesshore your statements are exact according to my knowledge. Regarding (5) it is partly-true only as people can still require an exact file inside a module, ex.: |
Would it be possible for us to add support for package.json to the Preprocessor? In other words, preprocessors: {
'**/*.js': ['commonjs'],
'**/package.json': ['commonjs']
} Then the Preprocessor would parse any package.json files it finds, add the |
+1 for resolving package.json. |
I'm just running into this issue today, so a bit late to the party, but hopefully some of this is useful... @pkozlowski-opensource regarding your last comment, requiring an npm module by using the full path doesn't work either, e.g. I'm doing
All of those paths/files exist and are valid. The relevant section of
Let me know if you want any further info. Cheers. |
@jazlalli could you put together a minimal project somewhere on GitHub demonstrating that full path resolution doesn't work? |
@jazlalli It looks like you need to add socket.io to the preprocessors block, i.e.,
|
hi folks, thanks for your replies @jamesshore unfortunately, no change after adding socket.io to preprocessors, still throws the same error. @pkozlowski-opensource I'll try and throw something together shortly. Will get back to you when I have something. Cheers |
I have experienced this issue as well. +1 for a fix as I think there is serious value in implementing this. |
+1 for fixing this. I could partially make it work for some packages hacking the config. E.g: files: [
'client/*.js',
'test/client/*.js',
'node_modules/..../file.js
],
preprocessors: [
'client/*.js': ['commonjs'],
'test/client/*.js': ['commonjs'],
'node_modules/..../file.js: ['commonjs']
], This will work for packages where the filename is the same as the package name. E.g.
and in the package (inside node_modules) there is a file x.js This is hacky and it won't work for all the packages. Looking at the Karma project though, they are using karma-commonjs and it seems to work, so not sure what's going on. |
Unfortunately can't get this to work in I get an
The error happens in the But after I load it like this:
It throws that
Whereas other preprocessed JS files all look like this:
Have I missed something? How is this supposed to be configured? Thanks a lot in advance for your help. |
I finally got However this line is now making trouble:
I suppose it is usable on Unix/Linux (though I can't clearly imagine why), but on Windows it strips the " |
@givankin did you ever resolve this? It's a year on and I'm going round in circles hitting the same issues and also got as far as stepping through the code to see what was going on and came to the same conclusion about the / stripping. |
@DaveEmmerson The Windows issue is a bug, which I fixed in #54. Just waiting for @pkozlowski-opensource to merge it. |
@DaveEmmerson what I ended up doing is creating a (pretty complicated) set of grunt tasks that assemble all the dependencies for me and put into a special folder specifically for tests, and then I have
in my |
If one has a dependency that looks like this:
And in
package.json
:Then there is a
From a brief look at source code I suppose that
karma-commonjs
doesn't parsepackage.json
s. Maybe it's fine, doing it in the browser would be strange, but this means (correct me if I'm wrong) that I have to use built JS to run tests. And that's what I'd like to avoid because build procedure can grow complex and slow and running it on every "Shift+F10" makes people sad.What if a special "map" is introduced in config file, so we can do
or
somewhere, in the spirit of
browserify-shim
and similar libraries? This would also makekarma-commonjs
feasible in cases where the dependencies are not innode_modules
folder (e.g. ifbower
+debowerify
is used).The text was updated successfully, but these errors were encountered: