[node] switch to NodeRequest member fn callback

For (hopefully) better performance than creating a new v8::Context to
wrap each callback while still avoiding leaking memory with v8::FunctionTemplate.

Adds a JavaScript shim in front of module.exports.Map to wrap the
req.respond API internally and preserve the public callback-passing
API, while still exporting the correct prototype.

8 files changed, 83 insertions, 16 deletions

diff --git a/package.json b/package.json
index 1dd72807d7..e3982cbb3c 100644
--- a/package.json
+++ b/package.json
@@ -6,7 +6,7 @@
     "mapbox",
     "gl"
   ],
-  "main": "lib/mapbox-gl-native.node",
+  "main": "platform/node/index.js",
   "repository": {
     "type": "git",
     "url": "git://github.com/mapbox/mapbox-gl-native.git"
diff --git a/platform/node/index.js b/platform/node/index.js
new file mode 100644
index 0000000000..8c59023847
--- /dev/null
+++ b/platform/node/index.js
@@ -0,0 +1,31 @@
+"use strict";
+
+// Shim to wrap req.respond while preserving callback-passing API
+
+var mbgl = require('../../lib/mapbox-gl-native.node');
+var constructor = mbgl.Map.prototype.constructor;
+
+var Map = function(options) {
+    if (!(options instanceof Object)) {
+        throw TypeError("Requires an options object as first argument");
+    }
+
+    if (!options.hasOwnProperty('request') || !(options.request instanceof Function)) {
+        throw TypeError("Options object must have a 'request' method");
+    }
+
+    var request = options.request;
+
+    return new constructor(Object.assign(options, {
+        request: function(req) {
+            request(req, function() {
+                req.respond.apply(req, arguments);
+            });
+        }
+    }));
+};
+
+Map.prototype = mbgl.Map.prototype;
+Map.prototype.constructor = Map;
+
+module.exports = Object.assign(mbgl, { Map: Map });
diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp
index a563264db5..1fc9e987c7 100644
--- a/platform/node/src/node_map.cpp
+++ b/platform/node/src/node_map.cpp
@@ -43,11 +43,11 @@ static const char* releasedMessage() {
     return "Map resources have already been released";
 }
 
-NAN_MODULE_INIT(NodeMap::Init) {
+void NodeMap::Init(v8::Local<v8::Object> target) {
     v8::Local<v8::FunctionTemplate> tpl = Nan::New<v8::FunctionTemplate>(New);
 
-    tpl->InstanceTemplate()->SetInternalFieldCount(2);
     tpl->SetClassName(Nan::New("Map").ToLocalChecked());
+    tpl->InstanceTemplate()->SetInternalFieldCount(2);
 
     Nan::SetPrototypeMethod(tpl, "load", Load);
     Nan::SetPrototypeMethod(tpl, "loaded", Loaded);
diff --git a/platform/node/src/node_map.hpp b/platform/node/src/node_map.hpp
index 3673b5f2ba..5f6176ba75 100644
--- a/platform/node/src/node_map.hpp
+++ b/platform/node/src/node_map.hpp
@@ -23,7 +23,7 @@ public:
 
     static Nan::Persistent<v8::Function> constructor;
 
-    static NAN_MODULE_INIT(Init);
+    static void Init(v8::Local<v8::Object> exports);
 
     static void New(const Nan::FunctionCallbackInfo<v8::Value>&);
     static void Load(const Nan::FunctionCallbackInfo<v8::Value>&);
diff --git a/platform/node/src/node_request.cpp b/platform/node/src/node_request.cpp
index aa84147ad6..607e6e8b03 100644
--- a/platform/node/src/node_request.cpp
+++ b/platform/node/src/node_request.cpp
@@ -32,6 +32,8 @@ NAN_MODULE_INIT(NodeRequest::Init) {
     tpl->InstanceTemplate()->SetInternalFieldCount(1);
     tpl->SetClassName(Nan::New("Request").ToLocalChecked());
 
+    Nan::SetPrototypeMethod(tpl, "respond", HandleCallback);
+
     constructor.Reset(tpl->GetFunction());
 
     // TODO: Remove this from the public JavaScript API
@@ -49,8 +51,9 @@ void NodeRequest::New(const Nan::FunctionCallbackInfo<v8::Value>& info) {
 }
 
 void NodeRequest::HandleCallback(const Nan::FunctionCallbackInfo<v8::Value>& info) {
+    auto request = Nan::ObjectWrap::Unwrap<NodeRequest>(info.Holder());
+
     // Move out of the object so callback() can only be fired once.
-    auto request = Nan::ObjectWrap::Unwrap<NodeRequest>(info.Data().As<v8::Object>());
     auto callback = std::move(request->callback);
     if (!callback) {
         return info.GetReturnValue().SetUndefined();
@@ -122,14 +125,9 @@ void NodeRequest::HandleCallback(const Nan::FunctionCallbackInfo<v8::Value>& inf
 }
 
 void NodeRequest::Execute() {
-    // Enter a new v8::Context to avoid leaking v8::FunctionTemplate
-    // from Nan::New<v8::Function>
-    v8::Local<v8::Context> context = v8::Context::New(v8::Isolate::GetCurrent());
-    v8::Context::Scope scope(context);
-
-    v8::Local<v8::Value> argv[] = { handle(), Nan::New<v8::Function>(NodeRequest::HandleCallback, handle()) };
+    v8::Local<v8::Value> argv[] = { handle() };
 
-    Nan::MakeCallback(Nan::To<v8::Object>(target->handle()->GetInternalField(1)).ToLocalChecked(), "request", 2, argv);
+    Nan::MakeCallback(Nan::To<v8::Object>(target->handle()->GetInternalField(1)).ToLocalChecked(), "request", 1, argv);
 }
 
 NodeRequest::NodeAsyncRequest::NodeAsyncRequest(NodeRequest* request_) : request(request_) {
diff --git a/platform/node/test/js/map.test.js b/platform/node/test/js/map.test.js
index 6ffbf3eb66..9300cac4e1 100644
--- a/platform/node/test/js/map.test.js
+++ b/platform/node/test/js/map.test.js
@@ -1,13 +1,15 @@
 'use strict';
 
 var test = require('tape');
-var mbgl = require('../../../../lib/mapbox-gl-native');
+var mbgl = require('../../index');
 var fs = require('fs');
 var path = require('path');
 var style = require('../fixtures/style.json');
 
 test('Map', function(t) {
-    t.test('must be constructed with new', function(t) {
+    // This test is skipped because of the req.respond shim in index.js
+    // which will always call new mbgl.Map()
+    t.skip('must be constructed with new', function(t) {
         t.throws(function() {
             mbgl.Map();
         }, /Use the new operator to create new Map objects/);
@@ -87,6 +89,42 @@ test('Map', function(t) {
         t.end();
     });
 
+    t.test('instanceof mbgl.Map', function(t) {
+        var options = {
+            request: function() {},
+            ratio: 1
+        };
+
+        var map = new mbgl.Map(options);
+
+        t.ok(map instanceof mbgl.Map);
+        t.equal(map.__proto__, mbgl.Map.prototype);
+
+        var keys = Object.keys(mbgl.Map.prototype);
+
+        t.deepEqual(keys, [
+            'load',
+            'loaded',
+            'render',
+            'release',
+            'addClass',
+            'addSource',
+            'addLayer',
+            'removeLayer',
+            'setLayoutProperty',
+            'setPaintProperty',
+            'setFilter',
+            'dumpDebugLogs',
+            'queryRenderedFeatures'
+        ]);
+
+        for (var key in keys) {
+            t.equal(map[key], mbgl.Map.prototype[key]);
+        }
+
+        t.end();
+    });
+
     t.test('.load', function(t) {
         var options = {
             request: function() {},
diff --git a/platform/node/test/memory.test.js b/platform/node/test/memory.test.js
index e958f8a29b..ad54098832 100644
--- a/platform/node/test/memory.test.js
+++ b/platform/node/test/memory.test.js
@@ -1,7 +1,7 @@
 'use strict';
 
 var fs = require('fs');
-var mbgl = require('../../../lib/mapbox-gl-native');
+var mbgl = require('../index');
 var path = require('path');
 var test = require('tape');
 
diff --git a/platform/node/test/suite_implementation.js b/platform/node/test/suite_implementation.js
index 05c2da0a6d..bde8acde18 100644
--- a/platform/node/test/suite_implementation.js
+++ b/platform/node/test/suite_implementation.js
@@ -1,6 +1,6 @@
 'use strict';
 
-var mbgl = require('../../../lib/mapbox-gl-native');
+var mbgl = require('../index');
 var request = require('request');
 
 mbgl.on('message', function(msg) {