Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Introduce NODE_MODULE_INIT() to pass full `module` object to addons #4634

Closed
wants to merge 2 commits into from

9 participants

@rvagg

Proposal to allow native addons have access to the full module object so they can optionally overwrite exports, with a single function for example. Currently they are restricted to operating on an existing object.

Keeps NODE_MODULE() intact so doesn't affect existing addons, introduces NODE_MODULE_INIT() as an alternative and perhaps NODE_MODULE() could be deprecated and replaced with NODE_MODULE_INIT() in the docs with

NODE_SET_METHOD(target->Get(String::New("exports")), "foo", Foo);

(or a new, more convenient macro perhaps?).

Thoughts?

@rvagg rvagg referenced this pull request in Level/leveldown
Closed

export single function, move iterator() to instance #5

src/node.h
@@ -202,7 +202,8 @@ struct node_module_struct {
int version;
void *dso_handle;
const char *filename;
- void (*register_func) (v8::Handle<v8::Object> target);
+ void (*register_func) (v8::Handle<v8::Object> exports);
+ void (*module_register_func) (v8::Handle<v8::Object> module);

Bump NODE_MODULE_VERSION, you're changing the struct's size and layout.

@rvagg
rvagg added a note

fixed 051945b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/node.cc
@@ -1869,7 +1869,11 @@ static gid_t gid_by_name(Handle<Value> value) {
}
// Execute the C++ module
- mod->register_func(target);
+ if (mod->register_func != NULL) {
+ mod->register_func(target->Get(String::New("exports"))->ToObject());

I'd probably split this up and add a sanity check:

Local<Value> exports = target->Get(String::New("exports"));
assert(exports.IsObject());
mod->register_func(exports.As<Object>());
@rvagg
rvagg added a note

@bnoordhuis is this necessary since the call comes in from module.js where exports is explicitly defined?

Defense in depth. Stuff like this helps catching regressions.

@TooTallNate Owner
@rvagg
rvagg added a note

ok, probably needs a new name to better indicate the difference, any suggestions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bnoordhuis

The PR itself mostly LGTM but I kind of question its usefulness. You can accomplish the same thing with a JS shim, right?

@indutny
Owner

I agree with @bnoordhuis, in the most of cases you'll need to wrap it up in js-land to do some checks and create rich api.

@rvagg

This comes out of discussion on LevelDOWN where we want to have an almost completely native LevelDB addon that we can extend with LevelUP. We'd like to export a single function; this is emerging as the dominant pattern for Node modules so it's really a bit of an oversight that this is not possible from .node modules. Of course we can (and will) deal with it for now by exporting a function from the exports in the .node and then assigning that function as the exports of the main .js but it's a bit of a smell that you have to do it like that.

Also, this isn't just about being able to overwrite module.exports, there's more on module that would be good to have access to in native modules. And, surely this is a matter of consistency? Why only give .node modules the exports object but give .js modules the whole lot?

@bnoordhuis

@isaacs and @TooTallNate, you should probably chime in. Is this something people complain about?

@defunctzombie

-1, I don't think you should ever be exposing your native modules directly to users. Just make a js file which exposes them however you want and have that be what users (or you) require.

@TooTallNate
Owner
@juliangruber

+1 I don't see a reason against this

@rvagg

@shtylman this isn't about exposing them directly to users; you can't be sure where the .node will end up anyway (e.g. Debug).

@defunctzombie

What is the advantage of having this over just attaching to the exports object and letting the js layer do the rest? To me it just seems like more ways to do the same thing. You can already export just a function from js, so wrap your binding and export that.

@isaacs
Owner

If it's an advantage when you expose it to users, then it's an advantage when you're the user. "Export one thing" is generally better.

I'm +1 on this feature, whenever @bnoordhuis or @TooTallNate is happy with the code.

src/node.h
((5 lines not shown))
#define NODE_MODULE(modname, regfunc) \
extern "C" { \
NODE_MODULE_EXPORT node::node_module_struct modname ## _module = \
{ \
NODE_STANDARD_MODULE_STUFF, \
regfunc, \
+ NULL, \
+ NODE_STRINGIFY(modname) \
+ }; \
+ }
+
+// regfunc(module) form
+#define NODE_MODULE_INIT(modname, regfunc) \

Is there a better name for this macro? NODE_MODULE_INIT seems a little confusing; it suggests that NODE_MODULE doesn't perform initialization.

Also, it should be documented in doc/api/addons.markdown.

@rvagg
rvagg added a note

@bnoordhuis the reason I chose that name is that I imagine this being the primary interface for native addons with NODE_MODULE being deprecated. I'll do some work on addons.markdown and you can let me know what you think about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rvagg

Updated addons.markdown to use NODE_MODULE_INIT and also a note about NODE_MODULE being the "old" form. How does it sound?

@rvagg

I've added the sanity assert() and I'll tweak the docs some more if we're going to keep both NODE_MODULE and this new one around; but it probably needs a new name since NODE_MODULE_INIT doesn't really tell you much about how it's different from NODE_MODULE. Any suggestions for a name that may indicate that it passes module rather than just exports?

doc/api/addons.markdown
@@ -93,6 +97,13 @@ the recently built `hello.node` module:
Please see patterns below for further information or
<https://github.com/arturadib/node-qt> for an example in production.
+### `NODE_MODULE_INIT` vs `NODE_MODULE`
+
+In versions of Node prior to v0.10 the macro `NODE_MODULE` was used in place of
+`NODE_MODULE_INIT`; this macro would pass the `exports` object directly to your
+initialization function rather than the full `module` object. `NODE_MODULE` is
+still available but is deprecated and may be removed from a future version of Node.

I'm sorry for interrupting discussions, but I doubt it is necessary to remove NODE_MODULE macro in the future.
Anyway, it should not be put on until removing is settled, I think.

@rvagg
rvagg added a note

np @Lewuathe, it's in the process of being changed as @TooTallNate has pointed out that he'd rather it not be marked as deprecated (the intention of the message was just to encourage people not to use it, not to say that it will be removed, just give flexibility to remove it at some point in time).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rvagg

Have opted for NODE_MODULE_M unless someone comes up with a better idea. I've also changed the docs to point out the differences between the two forms, see here.

I also have the addon examples available in a repo here if anyone is interested in playing them in the current form (as per this branch).

@bnoordhuis

Have opted for NODE_MODULE_M unless someone comes up with a better idea.

NODE_MODULE_M is better but still a little je n'est ce quoi. Suggestions, anyone?

doc/api/addons.markdown
((18 lines not shown))
-There is no semi-colon after `NODE_MODULE` as it's not a function (see `node.h`).
+There is no semi-colon after `NODE_MODULE_M` as it's not a function (see `node.h`).

I'd rather you leave this section untouched. NODE_MODULE is not going away and is what I suspect most people will continue to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
doc/api/addons.markdown
@@ -93,6 +97,20 @@ the recently built `hello.node` module:
Please see patterns below for further information or
<https://github.com/arturadib/node-qt> for an example in production.
+### `NODE_MODULE` and `NODE_MODULE_M`
+
+There are two forms of the macro used to register your *init* function, you should use
+only one, depending on your preference or requirements:
+
+* When you register with <code><b>NODE_MODULE</b>(moduleName, registerFunction)</code>,
+your `registerFunction` will be passed the plain <b><code>exports</code></b> object
+when your module is initialized. You will then need to add your properties to this
+object to make them available to client code.
+* When you register with <code><b>NODE_MODULE_M</b>(moduleName, registerFunction)</code>,
+your `registerFunction` will be passed the full <b><code>`module`</code></b> object
+when your module is initialized. This then gives you the ability to completely
+overwrite the `exports` property if you require.

Use markdown here, not HTML.

@rvagg
rvagg added a note

was being cautious as some markdown parsers don't handle nested ` & ** properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rvagg

@bnoordhuis see what I mean about nested ` & **; GFM is crap at it but marked is better, which matters most I suppose.

@rvagg

hm, no, marked doesn't like that either; it gets the bolded exports and module right, but not the nested bolded NODE_MODULE and NODE_MODULE_M.

@rvagg

reverted to html to avoid ` & ** md confusion.

@isaacs isaacs addon: Pass module object to NODE_MODULE init function
mainly to allow native addons to export single functions on
rather than being restricted to operating on an existing
object.

Init functions now receive exports as the first argument, like
before, but also the module object as the second argument, if they
support it.

Related to #4634

cc: @rvagg
ede6592
@isaacs
Owner

What do you guys think of isaacs@ede6592? If we just cast to a function typedef, we can keep the macro the same, and pass in module as the second argument. Existing modules keep working without any changes, and we don't need a new macro name.

@rvagg

Nice, if that works cross platform the it's ideal. I'll update docs accordingly when I'm back at a computer.

@rvagg rvagg NODE_MODULE() to pass full `module` to addons
mainly to allow native addons to export single functions on `exports`
rather than being restricted to operating on an existing `exports`
object.

bump NODE_MODULE_VERSION for C++ API breakage
added link to addons repo in docs
bc7df2d
@rvagg

rebased to @isaacs branch, docs updated, looks good.

@isaacs isaacs referenced this pull request from a commit
@isaacs isaacs addon: Pass module object to NODE_MODULE init function
mainly to allow native addons to export single functions on
rather than being restricted to operating on an existing
object.

Init functions now receive exports as the first argument, like
before, but also the module object as the second argument, if they
support it.

Related to #4634

cc: @rvagg
1550858
@isaacs
Owner

Landed on master. Thanks @rvagg!

@isaacs isaacs closed this
@jugglinmike jugglinmike referenced this pull request in mozilla-b2g/sockit-to-me
Open

Use NodeJS 'exports' object #5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 24, 2013
  1. @isaacs

    addon: Pass module object to NODE_MODULE init function

    isaacs authored
    mainly to allow native addons to export single functions on
    rather than being restricted to operating on an existing
    object.
    
    Init functions now receive exports as the first argument, like
    before, but also the module object as the second argument, if they
    support it.
    
    Related to #4634
    
    cc: @rvagg
Commits on Jan 25, 2013
  1. @rvagg

    NODE_MODULE() to pass full `module` to addons

    rvagg authored
    mainly to allow native addons to export single functions on `exports`
    rather than being restricted to operating on an existing `exports`
    object.
    
    bump NODE_MODULE_VERSION for C++ API breakage
    added link to addons repo in docs
This page is out of date. Refresh to see the latest.
View
66 doc/api/addons.markdown
@@ -23,13 +23,15 @@ Node statically compiles all its dependencies into the executable. When
compiling your module, you don't need to worry about linking to any of these
libraries.
+All of the following examples are available for [download](https://github.com/rvagg/node-addon-examples)
+and may be used as a starting-point for your own Addon.
## Hello world
To get started let's make a small Addon which is the C++ equivalent of
the following JavaScript code:
- exports.hello = function() { return 'world'; };
+ module.exports.hello = function() { return 'world'; };
First we create a file `hello.cc`:
@@ -43,15 +45,16 @@ First we create a file `hello.cc`:
return scope.Close(String::New("world"));
}
- void init(Handle<Object> target) {
- target->Set(String::NewSymbol("hello"),
+ void init(Handle<Object> exports) {
+ exports->Set(String::NewSymbol("hello"),
FunctionTemplate::New(Method)->GetFunction());
}
+
NODE_MODULE(hello, init)
Note that all Node addons must export an initialization function:
- void Initialize (Handle<Object> target);
+ void Initialize (Handle<Object> exports);
NODE_MODULE(module_name, Initialize)
There is no semi-colon after `NODE_MODULE` as it's not a function (see `node.h`).
@@ -154,8 +157,8 @@ function calls and return a result. This is the main and only needed source
return scope.Close(num);
}
- void Init(Handle<Object> target) {
- target->Set(String::NewSymbol("add"),
+ void Init(Handle<Object> exports) {
+ exports->Set(String::NewSymbol("add"),
FunctionTemplate::New(Add)->GetFunction());
}
@@ -189,18 +192,23 @@ there. Here's `addon.cc`:
return scope.Close(Undefined());
}
- void Init(Handle<Object> target) {
- target->Set(String::NewSymbol("runCallback"),
+ void Init(Handle<Object> exports, Handle<Object> module) {
+ module->Set(String::NewSymbol("exports"),
FunctionTemplate::New(RunCallback)->GetFunction());
}
NODE_MODULE(addon, Init)
+Note that this example uses a two-argument form of `Init()` that receives
+the full `module` object as the second argument. This allows the addon
+to completely overwrite `exports` with a single function instead of
+adding the function as a property of `exports`.
+
To test it run the following JavaScript snippet:
var addon = require('./build/Release/addon');
- addon.runCallback(function(msg){
+ addon(function(msg){
console.log(msg); // 'hello world'
});
@@ -225,8 +233,8 @@ the string passed to `createObject()`:
return scope.Close(obj);
}
- void Init(Handle<Object> target) {
- target->Set(String::NewSymbol("createObject"),
+ void Init(Handle<Object> exports, Handle<Object> module) {
+ module->Set(String::NewSymbol("exports"),
FunctionTemplate::New(CreateObject)->GetFunction());
}
@@ -236,8 +244,8 @@ To test it in JavaScript:
var addon = require('./build/Release/addon');
- var obj1 = addon.createObject('hello');
- var obj2 = addon.createObject('world');
+ var obj1 = addon('hello');
+ var obj2 = addon('world');
console.log(obj1.msg+' '+obj2.msg); // 'hello world'
@@ -266,8 +274,8 @@ wraps a C++ function:
return scope.Close(fn);
}
- void Init(Handle<Object> target) {
- target->Set(String::NewSymbol("createFunction"),
+ void Init(Handle<Object> exports, Handle<Object> module) {
+ module->Set(String::NewSymbol("exports"),
FunctionTemplate::New(CreateFunction)->GetFunction());
}
@@ -278,7 +286,7 @@ To test:
var addon = require('./build/Release/addon');
- var fn = addon.createFunction();
+ var fn = addon();
console.log(fn()); // 'hello world'
@@ -294,8 +302,8 @@ module `addon.cc`:
using namespace v8;
- void InitAll(Handle<Object> target) {
- MyObject::Init(target);
+ void InitAll(Handle<Object> exports) {
+ MyObject::Init(exports);
}
NODE_MODULE(addon, InitAll)
@@ -309,7 +317,7 @@ Then in `myobject.h` make your wrapper inherit from `node::ObjectWrap`:
class MyObject : public node::ObjectWrap {
public:
- static void Init(v8::Handle<v8::Object> target);
+ static void Init(v8::Handle<v8::Object> exports);
private:
MyObject();
@@ -335,7 +343,7 @@ prototype:
MyObject::MyObject() {};
MyObject::~MyObject() {};
- void MyObject::Init(Handle<Object> target) {
+ void MyObject::Init(Handle<Object> exports) {
// Prepare constructor template
Local<FunctionTemplate> tpl = FunctionTemplate::New(New);
tpl->SetClassName(String::NewSymbol("MyObject"));
@@ -345,7 +353,7 @@ prototype:
FunctionTemplate::New(PlusOne)->GetFunction());
Persistent<Function> constructor = Persistent<Function>::New(tpl->GetFunction());
- target->Set(String::NewSymbol("MyObject"), constructor);
+ exports->Set(String::NewSymbol("MyObject"), constructor);
}
Handle<Value> MyObject::New(const Arguments& args) {
@@ -399,10 +407,10 @@ Let's register our `createObject` method in `addon.cc`:
return scope.Close(MyObject::NewInstance(args));
}
- void InitAll(Handle<Object> target) {
+ void InitAll(Handle<Object> exports, Handle<Object> module) {
MyObject::Init();
- target->Set(String::NewSymbol("createObject"),
+ module->Set(String::NewSymbol("exports"),
FunctionTemplate::New(CreateObject)->GetFunction());
}
@@ -490,14 +498,14 @@ The implementation is similar to the above in `myobject.cc`:
Test it with:
- var addon = require('./build/Release/addon');
+ var createObject = require('./build/Release/addon');
- var obj = addon.createObject(10);
+ var obj = createObject(10);
console.log( obj.plusOne() ); // 11
console.log( obj.plusOne() ); // 12
console.log( obj.plusOne() ); // 13
- var obj2 = addon.createObject(20);
+ var obj2 = createObject(20);
console.log( obj2.plusOne() ); // 21
console.log( obj2.plusOne() ); // 22
console.log( obj2.plusOne() ); // 23
@@ -533,13 +541,13 @@ In the following `addon.cc` we introduce a function `add()` that can take on two
return scope.Close(Number::New(sum));
}
- void InitAll(Handle<Object> target) {
+ void InitAll(Handle<Object> exports) {
MyObject::Init();
- target->Set(String::NewSymbol("createObject"),
+ exports->Set(String::NewSymbol("createObject"),
FunctionTemplate::New(CreateObject)->GetFunction());
- target->Set(String::NewSymbol("add"),
+ exports->Set(String::NewSymbol("add"),
FunctionTemplate::New(Add)->GetFunction());
}
View
4 lib/module.js
@@ -488,9 +488,7 @@ Module._extensions['.json'] = function(module, filename) {
//Native extension for .node
-Module._extensions['.node'] = function(module, filename) {
- process.dlopen(filename, module.exports);
-};
+Module._extensions['.node'] = process.dlopen;
// bootstrap main module.
View
23 src/node.cc
@@ -101,6 +101,8 @@ Persistent<String> domain_symbol;
static Persistent<Object> process;
+static Persistent<String> exports_symbol;
+
static Persistent<String> errno_symbol;
static Persistent<String> syscall_symbol;
static Persistent<String> errpath_symbol;
@@ -1786,8 +1788,8 @@ Handle<Value> Hrtime(const v8::Arguments& args) {
typedef void (UV_DYNAMIC* extInit)(Handle<Object> exports);
-// DLOpen is node.dlopen(). Used to load 'module.node' dynamically shared
-// objects.
+// DLOpen is process.dlopen(module, filename).
+// Used to load 'module.node' dynamically shared objects.
Handle<Value> DLOpen(const v8::Arguments& args) {
HandleScope scope;
char symbol[1024], *base, *pos;
@@ -1800,8 +1802,13 @@ Handle<Value> DLOpen(const v8::Arguments& args) {
return ThrowException(exception);
}
- String::Utf8Value filename(args[0]); // Cast
- Local<Object> target = args[1]->ToObject(); // Cast
+ Local<Object> module = args[0]->ToObject(); // Cast
+ String::Utf8Value filename(args[1]); // Cast
+
+ if (exports_symbol.IsEmpty()) {
+ exports_symbol = NODE_PSYMBOL("exports");
+ }
+ Local<Object> exports = module->Get(exports_symbol)->ToObject();
if (uv_dlopen(*filename, &lib)) {
Local<String> errmsg = String::New(uv_dlerror(&lib));
@@ -1812,7 +1819,7 @@ Handle<Value> DLOpen(const v8::Arguments& args) {
return ThrowException(Exception::Error(errmsg));
}
- String::Utf8Value path(args[0]);
+ String::Utf8Value path(args[1]);
base = *path;
/* Find the shared library filename within the full path. */
@@ -1869,7 +1876,7 @@ Handle<Value> DLOpen(const v8::Arguments& args) {
}
// Execute the C++ module
- mod->register_func(target);
+ mod->register_func(exports, module);
// Tell coverity that 'handle' should not be freed when we return.
// coverity[leaked_storage]
@@ -1953,7 +1960,9 @@ static Handle<Value> Binding(const Arguments& args) {
if ((modp = get_builtin_module(*module_v)) != NULL) {
exports = Object::New();
- modp->register_func(exports);
+ // Internal bindings don't have a "module" object,
+ // only exports.
+ modp->register_func(exports, Undefined());
binding_cache->Set(module, exports);
} else if (!strcmp(*module_v, "constants")) {
View
11 src/node.h
@@ -83,6 +83,7 @@
# endif
#endif
+
namespace node {
NODE_EXTERN extern bool no_deprecation;
@@ -198,11 +199,15 @@ NODE_EXTERN v8::Local<v8::Value> WinapiErrnoException(int errorno,
const char *signo_string(int errorno);
+
+NODE_EXTERN typedef void (* addon_register_func)(
+ v8::Handle<v8::Object> exports, v8::Handle<v8::Value> module);
+
struct node_module_struct {
int version;
void *dso_handle;
const char *filename;
- void (*register_func) (v8::Handle<v8::Object> target);
+ node::addon_register_func register_func;
const char *modname;
};
@@ -214,7 +219,7 @@ node_module_struct* get_builtin_module(const char *name);
* an API is broken in the C++ side, including in v8 or
* other dependencies.
*/
-#define NODE_MODULE_VERSION 0x000A /* v0.10 */
+#define NODE_MODULE_VERSION 0x000B /* v0.11 */
#define NODE_STANDARD_MODULE_STUFF \
NODE_MODULE_VERSION, \
@@ -232,7 +237,7 @@ node_module_struct* get_builtin_module(const char *name);
NODE_MODULE_EXPORT node::node_module_struct modname ## _module = \
{ \
NODE_STANDARD_MODULE_STUFF, \
- regfunc, \
+ (node::addon_register_func)regfunc, \
NODE_STRINGIFY(modname) \
}; \
}
View
15 test/addons/hello-world-function-export/binding.cc
@@ -0,0 +1,15 @@
+#include <node.h>
+#include <v8.h>
+
+using namespace v8;
+
+Handle<Value> Method(const Arguments& args) {
+ HandleScope scope;
+ return scope.Close(String::New("world"));
+}
+
+void init(Handle<Object> exports, Handle<Object> module) {
+ NODE_SET_METHOD(module, "exports", Method);
+}
+
+NODE_MODULE(binding, init);
View
8 test/addons/hello-world-function-export/binding.gyp
@@ -0,0 +1,8 @@
+{
+ 'targets': [
+ {
+ 'target_name': 'binding',
+ 'sources': [ 'binding.cc' ]
+ }
+ ]
+}
View
4 test/addons/hello-world-function-export/test.js
@@ -0,0 +1,4 @@
+var assert = require('assert');
+var binding = require('./build/Release/binding');
+assert.equal('world', binding());
+console.log('binding.hello() =', binding());
Something went wrong with that request. Please try again.