From 7f0047c2d52eb39c30ee49a6e5a9a3167ec6f54d Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 15 Jul 2011 14:05:01 -0700 Subject: [PATCH] Close #1348 Remove require.paths Module.globalPaths is still set to a read-only copy of the global include paths pulled off of the NODE_PATH environment variable. It's important to be able to inspect this, but modifying it no longer has any effect. --- doc/api/modules.markdown | 79 +++---------------- lib/module.js | 26 ++++-- test/fixtures/require-path/p1/bar.js | 29 ------- test/fixtures/require-path/p1/foo.js | 23 ------ test/fixtures/require-path/p2/bar.js | 23 ------ test/fixtures/require-path/p2/foo.js | 23 ------ test/simple/test-module-loading.js | 9 +-- .../simple/test-require-cache-without-stat.js | 12 +-- 8 files changed, 43 insertions(+), 181 deletions(-) delete mode 100644 test/fixtures/require-path/p1/bar.js delete mode 100644 test/fixtures/require-path/p1/foo.js delete mode 100644 test/fixtures/require-path/p2/bar.js delete mode 100644 test/fixtures/require-path/p2/foo.js diff --git a/doc/api/modules.markdown b/doc/api/modules.markdown index bed2801609d..1ceb87edfca 100644 --- a/doc/api/modules.markdown +++ b/doc/api/modules.markdown @@ -231,77 +231,24 @@ in pseudocode of what require.resolve does: c. let I = I - 1 6. return DIRS -### Loading from the `require.paths` Folders +### Loading from the global folders -In node, `require.paths` is an array of strings that represent paths to -be searched for modules when they are not prefixed with `'/'`, `'./'`, or -`'../'`. For example, if require.paths were set to: +If the `NODE_PATH` environment variable is set to a colon-delimited list +of absolute paths, then node will search those paths for modules if they +are not found elsewhere. - [ '/home/micheil/.node_modules', - '/usr/local/lib/node_modules' ] +Additionally, node will search in the following locations: -Then calling `require('bar/baz.js')` would search the following -locations: +* 1: `$HOME/.node_modules` +* 2: `$HOME/.node_libraries` +* 3: `$PREFIX/lib/node` -* 1: `'/home/micheil/.node_modules/bar/baz.js'` -* 2: `'/usr/local/lib/node_modules/bar/baz.js'` +Where `$HOME` is the user's home directory, and `PREFIX` is node's +configured `installPrefix`. -The `require.paths` array can be mutated at run time to alter this -behavior. - -It is set initially from the `NODE_PATH` environment variable, which is -a colon-delimited list of absolute paths. In the previous example, -the `NODE_PATH` environment variable might have been set to: - - /home/micheil/.node_modules:/usr/local/lib/node_modules - -Loading from the `require.paths` locations is only performed if the -module could not be found using the `node_modules` algorithm above. -Global modules are lower priority than bundled dependencies. - -#### **Note:** Please Avoid Modifying `require.paths` - -`require.paths` may disappear in a future release. - -While it seemed like a good idea at the time, and enabled a lot of -useful experimentation, in practice a mutable `require.paths` list is -often a troublesome source of confusion and headaches. - -##### Setting `require.paths` to some other value does nothing. - -This does not do what one might expect: - - require.paths = [ '/usr/lib/node' ]; - -All that does is lose the reference to the *actual* node module lookup -paths, and create a new reference to some other thing that isn't used -for anything. - -##### Putting relative paths in `require.paths` is... weird. - -If you do this: - - require.paths.push('./lib'); - -then it does *not* add the full resolved path to where `./lib` -is on the filesystem. Instead, it literally adds `'./lib'`, -meaning that if you do `require('y.js')` in `/a/b/x.js`, then it'll look -in `/a/b/lib/y.js`. If you then did `require('y.js')` in -`/l/m/n/o/p.js`, then it'd look in `/l/m/n/o/lib/y.js`. - -In practice, people have used this as an ad hoc way to bundle -dependencies, but this technique is brittle. - -##### Zero Isolation - -There is (by regrettable design), only one `require.paths` array used by -all modules. - -As a result, if one node program comes to rely on this behavior, it may -permanently and subtly alter the behavior of all other node programs in -the same process. As the application stack grows, we tend to assemble -functionality, and those parts interact in ways that are difficult to -predict. +These are mostly for historic reasons. You are highly encouraged to +place your dependencies localy in `node_modules` folders. They will be +loaded faster, and more reliably. ### Accessing the main module diff --git a/lib/module.js b/lib/module.js index a0476ca9cd0..5e3fd84d09d 100644 --- a/lib/module.js +++ b/lib/module.js @@ -43,7 +43,8 @@ Module._contextLoad = (+process.env['NODE_MODULE_CONTEXTS'] > 0); Module._cache = {}; Module._pathCache = {}; Module._extensions = {}; -Module._paths = []; +var modulePaths = []; +Module.globalPaths = []; Module.wrapper = NativeModule.wrapper; Module.wrap = NativeModule.wrap; @@ -217,7 +218,7 @@ Module._resolveLookupPaths = function(request, parent) { var start = request.substring(0, 2); if (start !== './' && start !== '..') { - var paths = Module._paths; + var paths = modulePaths; if (parent) { if (!parent.paths) parent.paths = []; paths = parent.paths.concat(paths); @@ -229,7 +230,7 @@ Module._resolveLookupPaths = function(request, parent) { if (!parent || !parent.id || !parent.filename) { // make require('./path/to/foo') work - normally the path is taken // from realpath(__filename) but with eval there is no filename - var mainPaths = ['.'].concat(Module._paths); + var mainPaths = ['.'].concat(modulePaths); mainPaths = Module._nodeModulePaths('.').concat(mainPaths); return [request, mainPaths]; } @@ -353,15 +354,23 @@ Module.prototype._compile = function(content, filename) { require.resolve = function(request) { return Module._resolveFilename(request, self)[1]; - } - require.paths = Module._paths; + }; + + Object.defineProperty(require, 'paths', { get: function() { + throw new Error('require.paths is removed. Use ' + + 'node_modules folders, or the NODE_PATH '+ + 'environment variable instead.'); + }}); + require.main = process.mainModule; + // Enable support to add extra extension types require.extensions = Module._extensions; require.registerExtension = function() { throw new Error('require.registerExtension() removed. Use ' + 'require.extensions instead.'); - } + }; + require.cache = Module._cache; var dirname = path.dirname(filename); @@ -475,7 +484,10 @@ Module._initPaths = function() { paths = process.env['NODE_PATH'].split(':').concat(paths); } - Module._paths = paths; + modulePaths = paths; + + // clone as a read-only copy, for introspection. + Module.globalPaths = modulePaths.slice(0); }; // bootstrap repl diff --git a/test/fixtures/require-path/p1/bar.js b/test/fixtures/require-path/p1/bar.js deleted file mode 100644 index c66438783c5..00000000000 --- a/test/fixtures/require-path/p1/bar.js +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -var path = require('path'); - -require.paths.unshift(path.join(__dirname, '../p2')); - -exports.foo = require('foo'); - -exports.expect = require(path.join(__dirname, '../p2/bar')); -exports.actual = exports.foo.bar; diff --git a/test/fixtures/require-path/p1/foo.js b/test/fixtures/require-path/p1/foo.js deleted file mode 100644 index b0a5a2a575c..00000000000 --- a/test/fixtures/require-path/p1/foo.js +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -require.paths.unshift(__dirname); -exports.bar = require('bar'); diff --git a/test/fixtures/require-path/p2/bar.js b/test/fixtures/require-path/p2/bar.js deleted file mode 100644 index 95470396910..00000000000 --- a/test/fixtures/require-path/p2/bar.js +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - - -exports.INBAR = __filename; diff --git a/test/fixtures/require-path/p2/foo.js b/test/fixtures/require-path/p2/foo.js deleted file mode 100644 index afd44014d70..00000000000 --- a/test/fixtures/require-path/p2/foo.js +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -require.paths.unshift(__dirname); -exports.bar = require('bar'); // surprise! this is not /p2/bar, this is /p1/bar diff --git a/test/simple/test-module-loading.js b/test/simple/test-module-loading.js index 53b1ad4494e..736a1e2e49a 100644 --- a/test/simple/test-module-loading.js +++ b/test/simple/test-module-loading.js @@ -143,15 +143,14 @@ require.extensions['.test'] = function(module, filename) { }; assert.equal(require('../fixtures/registerExt2').custom, 'passed'); -common.debug('load modules by absolute id, then change require.paths, ' + - 'and load another module with the same absolute id.'); -// this will throw if it fails. -var foo = require('../fixtures/require-path/p1/foo'); -assert.ok(foo.bar.expect === foo.bar.actual); assert.equal(require('../fixtures/foo').foo, 'ok', 'require module with no extension'); +assert.throws(function() { + require.paths; +}, /removed/, 'Accessing require.paths should throw.'); + // Should not attempt to load a directory try { require('../fixtures/empty'); diff --git a/test/simple/test-require-cache-without-stat.js b/test/simple/test-require-cache-without-stat.js index 9e6b0aa881a..7c85a4ba942 100644 --- a/test/simple/test-require-cache-without-stat.js +++ b/test/simple/test-require-cache-without-stat.js @@ -45,17 +45,20 @@ fs.stat = function() { }; // Load the module 'a' and 'http' once. It should become cached. -require.paths.push(common.fixturesDir); -require('a'); +require(common.fixturesDir + '/a'); +require('../fixtures/a.js'); +require('./../fixtures/a.js'); require('http'); console.log("counterBefore = %d", counter); var counterBefore = counter; -// Now load the module a bunch of times. +// Now load the module a bunch of times with equivalent paths. // stat should not be called. for (var i = 0; i < 100; i++) { - require('a'); + require(common.fixturesDir + '/a'); + require('../fixtures/a.js'); + require('./../fixtures/a.js'); } // Do the same with a built-in module @@ -67,4 +70,3 @@ console.log("counterAfter = %d", counter); var counterAfter = counter; assert.equal(counterBefore, counterAfter); -