search

karma-runner / karma-commonjs

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

fix: allow processing non-js files #19

Closed pkozlowski-opensource closed 10 years ago

pkozlowski-opensource commented 10 years ago

Before this change the plugin assumed that all CJS files are available under their original names, which is making it difficult to use with files compiled to JS from other languages (Coffee etc.). Coffee processor is changing a file name (https://github.com/karma-runner/karma-coffee-preprocessor/blob/master/index.js#L14) but CJS plugin was "blind" to those changes.

I'm not sure if this is the best fix and I can think of different approaches so would be grateful for a review. Also not fully aware of other consequences of changing a file name like this on other parts of Karma pipeline (caching etc.) so review is even more welcomed :-)

jamesshore commented 10 years ago

I don't see a problem with it, but I think @vojtajina would be the best person to review in this case.

vojtajina commented 10 years ago

I think this should be fine.

Another solution could be to make the default extension in commonjs-preprocessor configurable. The recommended approach is to require module id (eq. ./foo/bar), which is then normalized into ./foo/bar.js. So you could configure this extension to .coffee or .ts.

The solution in this PR wouldn't work, if you use custom transformName function in coffee-preprocessor (eg. foo.coffee -> foo.coffee.js). The other solution would work.

On the other hand, the other solution would not allow to mix coffee/js/ts in your project, which the solution in this PR does allow. So I think, this PR is actually a better solution.