Merge pull request #2299 from mapbox/node-request-callback

[node] change request semantics

package.json

@@ -18,7 +18,7 @@
  }
  ],
  "dependencies": {
- "nan": "nodejs/nan#a71301e0f62877ca017c4ebec7fbac9f4359038e",
+ "nan": "^2.0.9",
  "node-pre-gyp": "^0.6.9"
  },
  "bundledDependencies": [

platform/node/CHANGELOG.md

@@ -4,11 +4,13 @@
 - Requires an options object argument to `new mbgl.Map()`
  (with required `request` and optional `cancel` methods),
  drops `mbgl.FileSource`.
+- Changes `request` semantics to pass a second, callback argument instead
+ of needing to call `req.respond`.
 - Requires numerical `ratio` in `mbgl.Map` options argument.
  Map pixel ratio is now immutable and can no longer be set with
  render options.
 - Adds support for rendering v8 styles.
-- No longer load resources before a render request is made.
+- No longer loads resources before a render request is made.
 - Adds io.js v3.x support.
 
 # 1.1.3

platform/node/README.md

@@ -92,22 +92,22 @@ The `kind` is an enum and defined in [`mbgl.Resource`](https://github.com/mapbox
 
 It has no significance for anything but serves as a hint to your implemention as to what sort of resource to expect. E.g., your implementation could choose caching strategies based on the expected file type.
 
-THe `request` implementation should pass uncompressed data to `req.respond`. If you are downloading assets from a source that applies gzip transport encoding, the implementation must decompress the results before passing them on.
+The `request` implementation should pass uncompressed data to `callback`. If you are downloading assets from a source that applies gzip transport encoding, the implementation must decompress the results before passing them on.
 
 A sample implementation that reads files from disk would look like the following:
 
 ```js
 var map = new mbgl.Map({
- request: function(req) {
+ request: function(req, callback) {
  fs.readFile(path.join('base/path', req.url), function(err, data) {
- req.respond(err, { data: data });
+ callback(err, { data: data });
  });
  },
  ratio: 1.0
 });
 ```
 
-This is a very barebones implementation and you'll probably want a better implementation. E.g. it passes the url verbatim to the file system, but you'd want add some logic that normalizes `http` URLs. You'll notice that once your implementation has obtained the requested file, you have to deliver it to the requestee by calling `req.respond()`, which takes either an error object or `null` and an object with several settings:
+This is a very barebones implementation and you'll probably want a better implementation. E.g. it passes the url verbatim to the file system, but you'd want add some logic that normalizes `http` URLs. You'll notice that once your implementation has obtained the requested file, you have to deliver it to the requestee by calling `callback()`, which takes either an error object or `null` and an object with several settings:
 
 ```js
 {
@@ -125,14 +125,14 @@ var mbgl = require('mapbox-gl-native');
 var request = require('request');
 
 var map = new mbgl.Map({
- request: function(req) {
+ request: function(req, callback) {
  request({
  url: req.url,
  encoding: null,
  gzip: true
  }, function (err, res, body) {
  if (err) {
- req.respond(err);
+ callback(err);
  } else if (res.statusCode == 200) {
  var response = {};
 
@@ -142,9 +142,9 @@ var map = new mbgl.Map({
 
  response.data = body;
 
- req.respond(null, response);
+ callback(null, response);
  } else {
- req.respond(new Error(JSON.parse(body).message));
+ callback(new Error(JSON.parse(body).message));
  }
  });
  },
@@ -164,7 +164,7 @@ var request = require('request');
 var url = require('url');
 
 var map = new mbgl.Map({
- request: function(req) {
+ request: function(req, callback) {
  var opts = {
  url: req.url,
  encoding: null,
@@ -177,7 +177,7 @@ var map = new mbgl.Map({
 
  request(opts, function (err, res, body) {
  if (err) {
- req.respond(err);
+ callback(err);
  } else if (res.statusCode == 200) {
  var response = {};
 
@@ -187,9 +187,9 @@ var map = new mbgl.Map({
 
  response.data = body;
 
- req.respond(null, response);
+ callback(null, response);
  } else {
- req.respond(new Error(JSON.parse(body).message));
+ callback(new Error(JSON.parse(body).message));
  }
  });
  },

platform/node/src/node_file_source.cpp

@@ -75,11 +75,14 @@ void NodeFileSource::processAdd(const mbgl::Resource& resource) {
  queue->ref();
  }
 
- v8::Local<v8::Object> requestHandle = NodeRequest::Create(this, resource)->ToObject();
+ auto requestHandle = NodeRequest::Create(this, resource)->ToObject();
  pending.emplace(resource, requestHandle);
 
- v8::Local<v8::Value> argv[] = { requestHandle };
- Nan::MakeCallback(Nan::New(options), "request", 1, argv);
+ auto callback = Nan::GetFunction(Nan::New<v8::FunctionTemplate>(NodeRequest::Respond, requestHandle)).ToLocalChecked();
+ callback->SetName(Nan::New("respond").ToLocalChecked());
+
+ v8::Local<v8::Value> argv[] = { requestHandle, callback };
+ Nan::MakeCallback(Nan::New(options), "request", 2, argv);
 }
 
 void NodeFileSource::processCancel(const mbgl::Resource& resource) {

platform/node/src/node_request.cpp

@@ -19,8 +19,6 @@ NAN_MODULE_INIT(NodeRequest::Init) {
  tpl->InstanceTemplate()->SetInternalFieldCount(1);
  tpl->SetClassName(Nan::New("Request").ToLocalChecked());
 
- Nan::SetPrototypeMethod(tpl, "respond", Respond);
-
  constructor.Reset(tpl->GetFunction());
  Nan::Set(target, Nan::New("Request").ToLocalChecked(), tpl->GetFunction());
 }
@@ -55,7 +53,7 @@ v8::Handle<v8::Object> NodeRequest::Create(NodeFileSource* source, const mbgl::R
 }
 
 NAN_METHOD(NodeRequest::Respond) {
- auto nodeRequest = Nan::ObjectWrap::Unwrap<NodeRequest>(info.Holder());
+ auto nodeRequest = Nan::ObjectWrap::Unwrap<NodeRequest>(info.Data().As<v8::Object>());
 
  // Request has already been responded to, or was canceled, fail silently.
  if (!nodeRequest->resource) {

platform/node/test/js/map.test.js

@@ -147,9 +147,9 @@ test('Map', function(t) {
 
  t.test('.render', function(t) {
  var options = {
- request: function(req) {
+ request: function(req, callback) {
  fs.readFile(path.join(__dirname, '..', req.url), function(err, data) {
- req.respond(err, { data: data });
+ callback(err, { data: data });
  });
  },
  ratio: 1

platform/node/test/render.test.js

@@ -13,9 +13,9 @@ if (process.argv[1] === __filename && process.argv.length > 2) {
 suite.run('native', {tests: tests}, function (style, options, callback) {
  var map = new mbgl.Map({
  ratio: options.pixelRatio,
- request: function (req) {
+ request: function(req, callback) {
  request(req.url, {encoding: null}, function (err, response, body) {
- req.respond(err, {data: body});
+ callback(err, {data: body});
  });
  }
  });