Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Issues with path resolution illustrated #14

Closed
wants to merge 2 commits into from

2 participants

@ericmuyser

I'm not sure if this is already handled. I'm getting paths such as "/mymodule/lib/mydirectory/myfile" in require.modules, but within my module "./mydirectory/myfile.js" is being resolved to "/mymodule/mydirectory/myfile.js". Two problems:
1) Most module files reside in the lib subdirectory. I know it's not a standard, so it shouldn't be hardcoded, but I'm just illustrating the issue.
2) File extension isn't be handled. I use extensions explicitely, rather than omitting them. I'm sure others do as well. Ideally, the extension would be included in the require.modules list, but I can see how that would be an issue.

Thanks for the module

@substack
Owner

I've had some other reports of the require resolver failing. I'll try to make extensions more consistent too when I clean everything up.

@substack
Owner

Unfortunately this change breaks some tests.

substack : node-browserify $ expresso

   jade.js jade: Error: Cannot find module ''
    at Object.require (evalmachine.<anonymous>:10:21)
    at Function.fromFile (evalmachine.<anonymous>:22:12)
    at Object.<anonymous> (evalmachine.<anonymous>:1522:35)
    at Object.<anonymous> (evalmachine.<anonymous>:1527:18)
    at Object.<anonymous> (evalmachine.<anonymous>:1528:8)
    at require (evalmachine.<anonymous>:11:40)
    at evalmachine.<anonymous>:1:12
    at /home/substack/projects/node-browserify/test/jade.js:15:16
    at next (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:789:25)
    at runSuite (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:807:6)


   resolve.js resolve: AssertionError: "/foo/here" deepEqual ""
    at /home/substack/projects/node-browserify/test/resolve.js:15:12
    at next (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:789:25)
    at runSuite (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:807:6)
    at check (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:714:16)
    at runFile (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:718:10)
    at Array.forEach (native)
    at runFiles (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:695:13)
    at run (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:663:5)
    at Object.<anonymous> (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:875:13)
    at Module._compile (module.js:420:26)


   entry.js entry: Error: Cannot find module './'
    at require (evalmachine.<anonymous>:10:21)
    at Function.fromFile (evalmachine.<anonymous>:22:12)
    at evalmachine.<anonymous>:1542:35
    at evalmachine.<anonymous>:1545:19
    at evalmachine.<anonymous>:1547:2
    at /home/substack/projects/node-browserify/test/entry.js:12:8
    at next (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:789:25)
    at runSuite (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:807:6)
    at check (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:714:16)
    at runFile (/home/substack/prefix/lib/node_modules/expresso/bin/expresso:718:10)


   uncaught: Error: Cannot find module './'
    at Object.require (evalmachine.<anonymous>:10:21)
    at Function.fromFile (evalmachine.<anonymous>:22:12)
    at Object.<anonymous> (evalmachine.<anonymous>:1544:35)
    at Object.<anonymous> (evalmachine.<anonymous>:1548:19)
    at Object.<anonymous> (evalmachine.<anonymous>:1555:8)
    at require (evalmachine.<anonymous>:11:40)
    at evalmachine.<anonymous>:1:11
    at IncomingMessage.<anonymous> (/home/substack/projects/node-browserify/test/simple.js:84:20)
    at IncomingMessage.emit (events.js:81:20)
    at HTTPParser.onMessageComplete (http.js:133:23)


   Failures: 4
@substack
Owner

I just rolled out a quick fix for extensions but I'm not sure that the behavior you describe is compatible with the latest versions of node and npm. Now directories.lib is ignored in the package.json but relative requires are always with respect to the directory a file is located in so even if require.modules shows a script at somemodule/somedirectory/somefile.js, another script at somemodule/somedirectory/moo.js will be able to require('./somefile').

@ericmuyser

Well, I'm not sure. I know relative pathing and for some reason it's failing. When I had directories.lib set, it would set my require.modules paths to /lib/lib/ (double), so I removed that. The resolved path was still /path rather than /lib/path when require being called from /lib -- strange.

I can see what you intended for extension -- looking forward to the fix.

I'll try the fix. Thanks!

@ericmuyser

npm info using npm@0.3.18
npm info using node@v0.5.0-pre

@ericmuyser

Here's another one. Node.js allows recursive require. Currently, browserify tries to cache the first require, but if there's any recursion it will never set a cache, and loop forever. Node.js behavior is to set an module's export immediately as it is ready, and from what I can tell it won't re-require unless the context has changed. Quantifying if the context has changed is difficult in the browser, where each module has it's own copy of require. The best I could think of was to cache the exports based on the file doing the require, and the file it's requiring, but one of my apps actually has a global importer, so the require always comes from the same file.. and thus that does not work. Another way is to check if the amount of exports has changed, so that it can step through the requires, recursively requiring eachother until one of them doesn't have any exports to assign. That seems to work good for me. I'm not sure it would for everyone, but I don't see any alternative.

@ericmuyser

Essentially, I've just made the cache based on the amount of exports. In most cases the caching will remain the same, but now it works in the same way I currently use Node.js

@ericmuyser

Hi!

I was wondering if there is any known issue with Socket.IO? I was running into some issues with "cannot find module 'url'" in my version, so I updated. Now, I can't get Socket.IO included. I will try again tomorrow.
Uncaught Error: Cannot find module 'socket.io/lib/socket.io'

BTW, I noticed there is a pathing inconsistency:
__filename the module name is doubled, is that normal?

_browserifyRequire.modules["socket.io/lib/socket.io/utils.js"] = function () {
var module = { exports : {} };
var exports = module.exports;
var __dirname = "socket.io";
var __filename = "socket.io/socket.io/lib/socket.io/utils.js";

var require = function (path) {
    return _browserifyRequire.fromFile("socket.io/lib/socket.io/utils.js", path);
};

(function () {
    exports.options = {

options: function(options, merge){
this.options = exports.merge(options || {}, merge || {});
}
};

@ericmuyser

url error was due to trying to include server-side socket.io. My mistake.

There's still an issue when your index is not in the root directory of your module, but I opted to specify an index.js in the root, which does the require.

There's still an issue with incremental importing, and cache preventing normal Node behavior. I had to update mine to get ti working with 0.3.6. I'll submit a fix later.

@ericmuyser ericmuyser closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 17 additions and 5 deletions.
  1. +13 −4 wrappers/body.js
  2. +4 −1 wrappers/prelude.js
View
17 wrappers/body.js
@@ -1,6 +1,13 @@
-_browserifyRequire.modules[$filename] = function () {
- var module = { exports : {} };
+_browserifyRequire.modules[$filename] = function (callee) {
+ var module = _browserifyRequire.modules[$filename]._cached;
var exports = module.exports;
+ var total = _browserifyRequire.modules[$filename]._totalExports;
+
+ _browserifyRequire.modules[$filename]._totalExports = module.exports.length;
+
+ if(total === module.exports.length)
+ return module.exports;
+
var __dirname = $__dirname;
var __filename = $__filename;
@@ -11,7 +18,9 @@ _browserifyRequire.modules[$filename] = function () {
(function () {
$body;
}).call(module.exports);
-
- _browserifyRequire.modules[$filename]._cached = module.exports;
+
return module.exports;
};
+
+_browserifyRequire.modules[$filename]._cached = { exports : [] };
+_browserifyRequire.modules[$filename]._totalExports = -1;
View
5 wrappers/prelude.js
@@ -56,6 +56,9 @@ require.resolve = function (basefile, file) {
var n = basedir.match(/\//)
? basedir.replace(/[^\/]+$/,'') + norm
- : norm.replace(/^\.\//, basedir + '/');
+ : norm.replace(/^\.\//, basedir + '/lib/');
+
+ n = n.substr(0, n.lastIndexOf('.'));
+
return n.replace(/\/.\//, '/');
};
Something went wrong with that request. Please try again.