search

karma-runner / karma-commonjs

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

Update docs to reflect use cases / shortcomings #37

Closed bendrucker closed 10 years ago

bendrucker commented 10 years ago

The wild world of karma + browserify is finally being tamed: https://github.com/Nikku/karma-bro/issues/37. The short version of the upcoming changes is that karma-bro is becoming karma-browserify (replacing the existing package there) and all other karma/browserify modules will be deprecated. karma-bro is quite stable and supports the browserify API extremely well.

I wanted to open a discussion about deprecating this module, especially after noticing last week that many of the angular-hint* modules are using karma-bro. This is a bit of a different situation than the other karma-bro* modules because the use cases are overlapping but not identical. I think it's safe to say that enough people are making use of Browserify features like module resolution, transforms, and plugins to severely diminish the usefulness of a simple CommonJS implementation. Offering the sort of behavior people expect would require a massive duplication of effort (e.g. https://github.com/karma-runner/karma-commonjs/issues/29#issuecomment-50266115).

cc @nikku

pkozlowski-opensource commented 10 years ago

Well, honestly I don't see much point of depreciating this module. Personally I use it in several projects that use CommonJS as modules format but don't use Browserify as module loader / packager. There is more to CommonJS in a browser than Browserify.

We can definitively add a note to the readme to point people to the karma-browserify or whatever other solution people come up with for using karma + browserify, but this doesn't mean that this module should go away - it is named karma-commonjs and does what it advertises.

bendrucker commented 10 years ago

Fair point. If you're using it for cases outside of Browserify there's no need to deprecate.

I'm more concerned with how the repo is positioned (docs, per your label). The readme describes karma-commonjs as a solution for people who use Browserify but it's definitely not the best tool for that job. Per #29 it's going to cause major headaches for people who try to use it for that purpose. We can only speculate, but I wouldn't be surprised if 95% of the people who currently install karma-commonjs are using browserify, if not higher.

I'll wait until the end of the week to write up a PR to give others who have thoughts the opportunity to chime in.

jamesshore commented 10 years ago

I'm inclined to agree that people use karma-commonjs when they want full Browserify support. (Hence the requests for things like node_modules support, which isn't really part of the CommonJS spec.)

The original motivation for karma-commonjs was that Karma reported bad stack traces when used with a Browserify bundle. Does karma-bro fix that?

pkozlowski-opensource commented 10 years ago

@bendrucker you are completely right in saying that this project - as it stands today - is not the best fit for people using Browserify. I'm all for making it clear in the docs and pointing people to the best-in-class karma + browserify integration.

Still, I think that this project has its use for people not using Browserify as the loader / packager. So let's just make it clear in the docs. PR is highly welcomed.

bendrucker commented 10 years ago

@jamesshore karma-bro supports source maps. The stack traces are still a little gross since the stack lines contain both the path/line/column of the bundle file and the actual source. But you do indeed get the proper source code file and line.

jamesshore commented 10 years ago

@pkozlowski-opensource I'm happy for you to make the call on how you want to deal with this one. It looks like karma-bro runs Browserify directly, whereas we re-create the Node 'require' algorithm. I think there's a place for both, and advantages/disadvantages to both as well.

pkozlowski-opensource commented 10 years ago

@jamesshore @bendrucker yes, I definitively think that there is a room for both and as I've stated before Browserify is not the only solution for the CommonJS in a browser (albeit the most popular one atm). For example, Karma itself is having its tests written using CommonJS module syntax and totally wouldn't make sense to force it to use Browserify.

I'm all for pointing Browserify users for the best-in-class integration with Karma so let's iterate on the pull request to get the wording right. So I'm closing this issue - let's move the discussion to the PR #38