karma-runner / karma-commonjs

A Karma plugin. Test CommonJS modules.
MIT License
73 stars 30 forks source link

Trouble loading from node_modules #29

Open maxwellpeterson-wf opened 10 years ago

maxwellpeterson-wf commented 10 years ago

I am currently using browserify to build tests & source before a karma test run and would really like to get away from that model so that we don't have to have the extra time of the browserify task as well as having more useful output from karma when doing things like code coverage reports.

This preprocessor is working great(!) for the source and test files, however I can not get it to load anything from node_modules.

var chai = require('chai'); Error: Could not find module 'chai' from '~/project_path/someSpec.js'

I've done some digging and logging in commonjs_bridge.js and the path and everything is resolving/normalizing correctly... it seems the issue is that none of the node_modules paths are in window.__cjs_module__. That seems to only be populated with files that I give to karma.

Should node_modules be there? Am I doing something wrong or is there potentially a bug in this? Thanks!

pkozlowski-opensource commented 10 years ago

@maxwellpeterson-wf the commonjs processor will only do its "magic" on the files included in the karma conf. So yes, if you want to have things from node_modules loaded, you need to add them to karma patterns as well.

Let me know if this info sorts out the problem for you. Otherwise you can try to put together a repo somewhere with a minimal reproduce scenario - I would be happy to have a look.

maxwellpeterson-wf commented 10 years ago

@pkozlowski-opensource I've played with it some more, and have it partially working for some things.

It works okay for node_modules that have a "bundled"/'dist" version of their module available like chai, if I include ./node_modules/chai/chai.js in the files given to karma. Even this is less than desirable because I would not like to modify that list of files everytime we add a node dependency.

However it is still an issue for something like React, which does not have a bundled dist file available. Because of this I would have to include all the of files for react (./node_modules/react/**/*.js) which is not going to work. Even if I do that, it fails because some of the files in react require some core nodejs modules and the preprocessor doesn't know where those are.

So it seems I am at a bit of a standstill on this front and will have to stick with browserify for now. I would still love to get this working though!

pkozlowski-opensource commented 10 years ago

@maxwellpeterson-wf I don't think we are aiming at providing support for native node modules. What we could think of is some kind of aliasing to point to browserify equivalents... The current CJS plugin will always have its limitations as it loads modules on the browser side and although we try to keep the loading algorithm as close as possible to the node's one we will always have some limitations (native modules, for example).

Not sure if I got an immediate remedy to your issues but if you are willing to dig a bit more into the details and pin-point particular problems, we can work on those.

necolas commented 10 years ago

Similar problem for me. Trying to port Flight to commonjs modules but can't get this preprocessor to work as desired when I npm-install the commonjs version of Flight in a project. I have to manually tell Karma about all the file paths of dependencie to preproccess, but while also avoiding other files in node_modules that I don't want because they'll blow up the runner. Pretty cumbersome and not something we can recommend to users of our framework.

pkozlowski-opensource commented 10 years ago

@necolas @maxwellpeterson-wf I'm all for doing it better, but I think we've got a kind-of-conflicting requirements here: (1) fast tests with minimal processing time, minimal amount of changes loaded into a browser etc. (2) easy setup

As of today the plugin is clearly optimised for (1) and I wonder how we could improve the (2) story without compromising the TDD experience.

@necolas maybe the way to go would be to write a karma extension for Flight which could load all the flight-related files? I would be willing to put some effort into this as I've got a similar use-case at work. Feel free to ping me here or via e-mail (it is public on GH) if you want to dive more into the topic -I might be able to help.

necolas commented 10 years ago

oh that's an interesting idea. but i guess the problem still exists once you start depending on other libraries in your app code and need to wire each of them up in karma's config.

pkozlowski-opensource commented 10 years ago

@necolas right, you would still have specify globs for other modules that you want to use. I think there is a general tension here between resolving CJS dependencies on the client and on the server. Since we want to run unit tests in a browser and run them fast those 2 options got those advantages / disadvantages (especially when it comes to working in the TDD mode):

Ideally we would like to have an algorithm, like browserify, that would take a set of entry-points (hmm, actually we've got them already - those are sources and tests) and would figure out all the dependencies from node_modules to load. This should be doable, provided that entry points use only "static" requires (that is, require("some_module") and not "dynamic" requires (require(someVarThatPointsToAModule)).

Yeh, this should be doable, probably an algorithm we want here is already available somewhere, as part of browserify or as a standalone module.

The idea

OK, so putting it all together - we could simplify configuration as follows:

I guess this would provide very easy setup for people and should perform reasonably well. I can see it being doable, the only blocking point for now would be https://github.com/karma-runner/karma/pull/948, but this one should be merged soon.

How does it sound?

maxwellpeterson-wf commented 10 years ago

This sounds great!

javoire commented 10 years ago

sounds great!

necolas commented 10 years ago

Sounds good!

we would have to extend the CJS framework to scan all the entry points and figure out all the dependencies that should be added to files served (but not watched!) by Karma

what would happen if one of the files in the tree changed? would it be re-served?

mzahor commented 10 years ago

Have the same issue with node_modules. +1 for the solution.

mightyiam commented 9 years ago

+1

loddit commented 8 years ago

+1 same problem to me.

AlexKovalevych commented 8 years ago

Getting a similar issue:

Could not find module 'jasmine-ajax'

Tried to add it to the files and preprocessor in karma conf, but it didn't work:

    files: [
        'src/*.js',
        'test/*Spec.js',
        './node_modules/jasmine-ajax/**/*.js'
    ],

...
    preprocessors: {
        'src/*.js': ['coverage', 'commonjs'],
        'test/*Spec.js': ['commonjs'],
        './node_modules/jasmine-ajax/**/*.js': ['commonjs']
    },

The test looks like:

require('jasmine-ajax');
....

Are there any ideas how to solve it?

AlexKovalevych commented 8 years ago

Got it working with browserify:

    files: [
        'test/*Spec.js',
    ],

    // preprocess matching files before serving them to the browser
    // available preprocessors: https://npmjs.org/browse/keyword/karma-preprocessor
    preprocessors: {
        'src/*.js': ['coverage', 'browserify'],
        'test/*Spec.js': ['browserify']
    },

...
    browserify: {
        debug: true,
        transform: [istanbul({
            ignore: ['**/node_modules/**', '**/test/**'],
        })],
    },
juanjoDiaz commented 6 years ago

Same problem here.

Will this be solved in the foreseeable future?