Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Make require a public property of module #1281

Closed
TrevorBurnham opened this issue Jul 6, 2011 · 6 comments
Closed

Make require a public property of module #1281

TrevorBurnham opened this issue Jul 6, 2011 · 6 comments
Labels
module-system

Comments

@TrevorBurnham
Copy link

When Node gives you a module instance, it comes with a require function. The two are linked to each other internally. So if you modify, for instance, module.filename, that affects the relative path used by require. But there's no way to get the require function corresponding to a given module (or vice versa). That's a pain if you want to do something like

sandbox = {module: new Module, require: /* what should go here? */);
vm.runInNewContext(code, sandbox);

This technique is used by CoffeeScript for its REPL, which led to some issues. The current workaround is to construct a new require that mimicks the one in the sandbox.module:

sandbox.require = _require = (path) -> Module._load path, _module
_require[r] = require[r] for r in Object.getOwnPropertyNames require
_require.paths = _module.paths = Module._nodeModulePaths process.cwd()
_require.resolve = (request) -> Module._resolveFilename request, _module

Obviously it'd be a lot less work to just write sandbox.require = sandbox.module.require.

Any reason not to expose require as a property of module? I believe this would just be a one-line change to module.js:

function require(path) {

to

self.require = function require(path) {
@ghost ghost assigned felixge Jul 9, 2011
@felixge
Copy link

felixge commented Jul 9, 2011

I would love to sneak this into 0.4.x. @ry @isaacs what do you think?

For now I have released two npm modules that adress these issues by using some ugly hacks:

@felixge felixge closed this as completed Jul 9, 2011
@felixge felixge reopened this Jul 9, 2011
@isaacs
Copy link

isaacs commented Jul 9, 2011

Yeah, it does have a certain elegant symmetry to module.exports. Not sure about putting it in v0.4, since it's new API, but it should be done, I think.

diff --git a/lib/module.js b/lib/module.js
index 11c5189..b725afa 100644
--- a/lib/module.js
+++ b/lib/module.js
@@ -336,6 +336,11 @@ Module.prototype.load = function(filename) {
 };


+Module.prototype.require = function(path) {
+  return Module._load(path, this);
+};
+
+
 // Returns exception if any
 Module.prototype._compile = function(content, filename) {
   var self = this;
@@ -343,7 +348,7 @@ Module.prototype._compile = function(content, filename) {
   content = content.replace(/^\#\!.*/, '');

   function require(path) {
-    return Module._load(path, self);
+    return self.require(path);
   }

   require.resolve = function(request) {

@isaacs
Copy link

isaacs commented Jul 9, 2011

To answer the "what goes here?" question in the op:

var module = new Module(".");
var sandbox = {module: module, require: function (p) { return Module._load(p, module) }, exports: module.exports };
vm.runInNewContext(code, sandbox);

@felixge
Copy link

felixge commented Jul 12, 2011

@isaacs: Ok, any reason for not landing this in 0.5 right now?

@isaacs
Copy link

isaacs commented Jul 14, 2011

@felixge Is that a +1 LGTM review? I'll add some tests and docs for it today.

@felixge
Copy link

felixge commented Jul 14, 2011

@isaacs: Yes, the patch looks great, can't await to have this : ).

@isaacs isaacs closed this as completed in 9b5098f Jul 14, 2011
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
module-system
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@isaacs @felixge @TrevorBurnham