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

Adds dubious fix for a possible error with loadTransform #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zachrose
Copy link

Hello @substack et al,

I have a issue that I don't totally understand and a fix that I don't totally understand either 😛. Perhaps by explaining the issue and showing the fix here someone can tell me what I've done wrong (or maybe what I've done right).

Here's the issue: I have an express app (let's call it B), that uses Browserify and the strictify plugin in a gulp task to bundle front-end JavaScript. Usually this works just fine. Then, for reasons I'd rather not get into, it made sense for B to be exported as a module into another express app (let's call it A), and used as middleware. (To make it's assets available, B has a postinstall script to run gulp.)

B has dependencies that are mostly non-overlapping with A's dependencies, except for underscore. As a result, running npm install on A puts underscore in A/node_modules and not in A/node_modules/B/node_modules.

In this arrangement, running the B's gulp task results in this error:

events.js:7
    throw er; // Unhandled 'error' event
        ^
Error: Cannot find module 'strictify' from '/home/zachrose/A/node_modules/underscore'
    at /home/zachrose/A/node_modules/B/node_modules/browserify/node_modules/resolve/lib/async.js:50:17
    at process (/home/zachrose/A/node_modules/B/node_modules/browserify/node_modules/resolve/lib/async.js:119:43)
    at /home/zachrose/A/node_modules/B/node_modules/browserify/node_modules/resolve/lib/async.js:128:21
    at load (/home/zachrose/A/node_modules/B/node_modules/browserify/node_modules/resolve/lib/async.js:60:43)
    at /home/zachrose/A/node_modules/B/node_modules/browserify/node_modules/resolve/lib/async.js:66:22
    at /home/zachrose/A/node_modules/B/node_modules/browserify/node_modules/resolve/lib/async.js:21:47
    at Object.oncomplete (fs.js:107:15)

After some trial and error, I discovered this change to module_deps, in addition to that adding {basedir: __dirname} as an option to my transform, like so:

    browserify({ /* yadda-yadda */}).transform('strictify', {basedir: __dirname})

...prevents the error and seems to make everything work ok. (This change doesn't fail the tests, but of course that doesn't mean it's the correct thing to do.)

So here are my questions:

  • What is the dirname option to b.transform supposed to do? Is it supposed to give a place to start looking for the transforming module (e.g. strictify?)
  • If not, is there another way I should be resolving my problem?

Thanks!

@zertosh
Copy link
Member

zertosh commented May 5, 2015

Hey @zachrose, is this something you're still interested in? Just asking to see if I should spend time on it or not since it's been 5 months 😞

@zachrose
Copy link
Author

zachrose commented May 6, 2015

Hey @zertosh, yeah I would be interested in having this looked at or merged. :)

@zertosh
Copy link
Member

zertosh commented May 6, 2015

So here's how it works: Remember that node's require resolves paths relative to the caller. Browserify tries really hard to behave this way. So when you pass in a string as the transform name, module-deps can't just say require(transformName); because it'll resolve relative to where module-deps is - not relative to the file you want transformed. That's why we have to fake out the whole resolve process, so the transform is loaded from the perspective of the transformed file. Since you're changing the basedir that's going to be used for resolution, it makes sense that it works for your unorthodox directory structure.

There is a way to avoid this whole process. Just pass in a transform function 😄 Have you tried this:

 browserify({ /* yadda-yadda */}).transform( require('strictify') );

I'm pretty sure that'll work. Let me know. I'd rather try to solve your problem given what's already available than add more complexity to the api.

@zachrose
Copy link
Author

zachrose commented May 6, 2015

Ah, lol. Thanks for the exposition. I'll try that tomorrow.

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

Successfully merging this pull request may close these issues.

2 participants