Skip to content
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

Trouble loading from node_modules #29

Open
maxwellpeterson-wf opened this issue Jul 9, 2014 · 16 comments
Open

Trouble loading from node_modules #29

maxwellpeterson-wf opened this issue Jul 9, 2014 · 16 comments

Comments

@maxwellpeterson-wf
Copy link

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
Copy link
Member

@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
Copy link
Author

@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
Copy link
Member

@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
Copy link

necolas commented Jul 23, 2014

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
Copy link
Member

@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
Copy link

necolas commented Jul 23, 2014

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
Copy link
Member

@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):

  • in a browser: each file can be processed / downloaded individually. This makes tests faster as we need to process / re-load only one (changed) file. The downside of this is that we need to specify which files to load into a browser;
  • on the server :each file change (test or src) means re-packaging / re-bundling and loading into a browser one (potentially big) file that can't be effectively cached. This slows down tests (time to bundle + time to download a file into a browser) but probably is easier when it comes to CJS modules resolution.

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:

  • users would have to specify only "entry point" paths in their Karma conf
  • 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

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 karma-runner/karma#948, but this one should be merged soon.

How does it sound?

@maxwellpeterson-wf
Copy link
Author

This sounds great!

@javoire
Copy link

javoire commented Aug 7, 2014

sounds great!

@necolas
Copy link

necolas commented Aug 7, 2014

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
Copy link

mzahor commented Aug 29, 2014

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

@mightyiam
Copy link

+1

lencioni added a commit to civiccc/react-waypoint that referenced this issue Jan 27, 2015
When picking a testing setup, we first took a swing at setting up Jest.
This went pretty well until we ran into a problem with offsetParent and
offsetTop being undefined on all nodes. We believe this is because Jest
tests aren't run within an actual browser environment. We considered
mocking the browser behavior required for this component, but that
seemed too painful and brittle. Since this component is so tied to the
DOM, we decided that it made more sense to run our tests in an actual
browser.

So, I decided to give Karma a try. Karma is a framework-agnostic test
runner that can run tests in multiple browsers.

  http://karma-runner.github.io/

Since we are using CommonJS, we needed something that would resolve our
require statements. I first tried setting up karma-commonjs plugin since
it seemed pretty straightforward, however, I ran into a problem when
trying to require a dependency from node_modules. It turns out that this
plugin was designed to mimic basic Node CommonJS behavior, which does
not include looking in node_modules. More info:

- karma-runner/karma-commonjs#29
- karma-runner/karma-commonjs#37

So, I decided to try out karma-webpack and I was up and running in no
time. I borrowed most of the configuration from react-router. There are
some options in there for running in CI that we'll likely end up
tweaking when we get CI actually set up, but I anticipate that we will
be doing that soon so it seemed reasonable to start that work here.
@loddit
Copy link

loddit commented Mar 24, 2016

+1 same problem to me.

@AlexKovalevych
Copy link

AlexKovalevych commented Aug 4, 2016

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
Copy link

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
Copy link

Same problem here.

Will this be solved in the foreseeable future?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants