bendrucker/proxyquire-universal

Cannot find module error with Coffeescript tests

alanrubin opened this issue · 10 comments

Based on issue #7 I have forked the demo repository (https://github.com/alanrubin/karma-browserify-proxyquire-issue) to support tests written in coffeescript. When trying to run, I get

Uncaught Error: Cannot find module '../src/sum/sum'
  at /var/folders/hh/cvg8rlfx4msgy3kv127zzdjr0000gn/T/4b7aa4ba4eaa93afbc6ac2ffbf5cfd54.browserify:54:0 <- /Users/alanrubin/keytree/BP/muse/coffee-issue/node_modules/proxyquireify/lib/prelude.js:54:0

If I add some console.logs from prelude.js:38 with the local "name" variable (used in currentCache[name]), I see that those proxyquired coffeescript resources for some reason still have a relative path and are not converted to an absolute path.

You can reproduce the issue with master branch.

Any ideas ? Thanks !

This is totally unrelated. You never load any sort of CoffeeScript compiler or register the .coffee extension so as far as Node is concerned those modules don't exist.

You npm test script is wrong...

Hang on

Seems that you can proxyquire coffeescript from JS without an issue. And you can run the whole thing in Node without issues. So I assume the coffee compiler is somehow screwing with things. If you can dig up a solution, we'd happily take a PR over at proxyquireify for it. Otherwise this isn't something I'm going to have time to investigate further.

First thanks for the quick reply ! Yes you are right I can proxyquire coffeescript from JS, that works fine. I have compared the browserify output for the JS and Coffee and indeed the require code inserted by proxyquire is missing in the Coffee version (after it is converted to JS).

That is very strange, I'm far from being a browserify expert but I was expecting that after applying a transform to the coffee file it would become a regular javascript file and then proxyquire plugin changes would be applied to the javascript. It seems for some reason something is being messed up in that order or the transform is being applied twice.

In fact the strangest thing is that I could not find a single example of browserify configuration mixing a plugin and a transform although this looks to me a valid scenario. Do you have any experience with that or can point me to an example ?

Mixing plugins and transforms is fine. First thing to investigate is what the coffeescript transform you're using writes out. As mentioned, I can't pick apart the CS stuff myself, but if you even get more info that would move us towards a solution go ahead and open a proxyquireify issue.

Thanks @bendrucker, I have investigate further and it seems I found 2 bugs in proxyquireify project. I'm not sure the best way to proceed, maybe I need to fill an issue there - @thlorenz that is probably relevant to you as well. Let me explain the two bugs.

  1. Unable to detect 'proxyquire = require('proxyquire')(require)'
    When Coffeescript runs through the script to convert to JS, the var declaration is created in the top of the file so the js code have the format
describe('sum - proxyquire', function() {
  var add, chai, expect, proxyquire, sinon, sum;
  // More code...
   proxyquire = require('proxyquire');
 // More code...
}

That is further converted by proxyquire-universal to

describe('sum - proxyquire', function() {
  var add, chai, expect, proxyquire, sinon, sum;
  // More code...
   proxyquire = require('proxyquireify')(require);
 // More code...
}

What I found is that https://github.com/thlorenz/proxyquireify/blob/master/lib/find-dependencies.js is unable to detect that syntax (it expects the var to be inlined with the declaration). The implementation I have described in the gist https://gist.github.com/alanrubin/a672f5bc0cc6c673a2b4 work in that case as well (requireWithImmediateCallWithoutVar method) but looks like we need to detect other cases (See my TODO comment there)

  1. Ignoring files without .js extension
    In https://github.com/thlorenz/proxyquireify/blob/master/lib/transform.js, you are testing if the file has .js extension, if not it is ignored. The problem is that my test file has .coffee extension even that it is now a JS (converted from Coffee but still maintains the original extension in the dependencies pipeline) so it is ignored. I have hardcoded .coffee test there, but I guess best approach will be to remove that test condition or use browserify "extensions" configuration to test against (see my TODO comment in the gist https://gist.github.com/alanrubin/a672f5bc0cc6c673a2b4).

How we should proceed to make sure those fixes are pushed to the project ? I would create a PR but there is still decisions to be made regarding the fixes, will be happy to get your opinion.

Definitely create issues and/or PRs over at proxyquireify for both of these. Appreciate you looking into these.

Re: extensions, I've always excluded json rather than trying to include only whitelisted extensions (whether by hardcoding or an option). I'm pretty sure json is one of the default extensions. You can always browserify stuff with other extensions, but that's going to have to come via a transform. At that point the user can set the appropriate order of transforms to make sure their files are valid JS (e.g. via partialify, brfs, etc.) by the time they hit the normal transform that's going to parse JS.

What I found is that https://github.com/thlorenz/proxyquireify/blob/master/lib/find-dependencies.js is unable to detect that syntax

@alanrubin this should be fixed in proxyquireify.
Could you please move that part of the issue over there and ideally submit a PR that makes it detect your case as well?

In https://github.com/thlorenz/proxyquireify/blob/master/lib/transform.js, you are testing if the file has .js extension, if not it is ignored

Same as above - proxyquireify issue. We need to allow overriding what extensions get ignored.
Then proxyquire-universal can hook into that -- unless proxyquire-universal is responsible for ignoring those extensions.

@bendrucker agree with you about the extensions whitelist, it seems a good solution, potentially anything with a different extension could be JS after being transpiled/converted from other language and JSON is the only extension that we must leave out.

@thlorenz will create a PR for those issues. I guess for issue (2) in transform.js we could follow @bendrucker recommendation and only ignore JSON or alternatively have that setup as an option in the transform. I will open an issue later in proxyquireify, let's follow from there.

Filed thlorenz/proxyquireify#24 and thlorenz/proxyquireify#25

PRs and tests appreciated on both