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

NodeRequest member function instead of v8::Context #5704

Merged
merged 1 commit into from
Sep 6, 2016

Conversation

mikemorris
Copy link
Contributor

Resurrects #5527, but with a JavaScript shim in front to preserve the callback-passing public API.

/cc @tmpsantos @jfirebaugh @miccolis

@mention-bot
Copy link

@mikemorris, thanks for your PR! By analyzing the annotation information on this pull request, we identified @kkaefer, @lucaswoj and @jfirebaugh to be potential reviewers

@mikemorris mikemorris added the Node.js node-mapbox-gl-native label Jul 15, 2016
@jfirebaugh
Copy link
Contributor

To bulletproof this for production, we should probably tweak the shim so that the exported Map.prototype has all the same properties as before, in case somebody is using Map.prototype.load.call(map, ...) or something like that.

I haven't actually tested it, but the following might work:

var Map = mbgl.Map;
var originalConstructor = Map.prototype.constructor;
Map.prototype.constructor = function(options) {
    ...
}

@mikemorris mikemorris force-pushed the node-request-respond branch 3 times, most recently from 3ba264e to 2408759 Compare September 2, 2016 21:08
@mikemorris
Copy link
Contributor Author

Rebased this and updated the shim to export the proper prototype, added tests (which were passing on master, failing here before but fixed now) to confirm expected behavior.

@mikemorris mikemorris force-pushed the node-request-respond branch 2 times, most recently from c0e6f0c to b49aedc Compare September 6, 2016 17:26
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.
@mikemorris mikemorris merged commit edec56e into master Sep 6, 2016
@mikemorris mikemorris deleted the node-request-respond branch September 7, 2016 00:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Node.js node-mapbox-gl-native
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants