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

[node] Added Nan::AsyncResource in NodeMap::renderFinished #11316

Closed
wants to merge 1 commit into from

Conversation

brunoabinader
Copy link
Member

Nan recently deprecated Callback::Call without a given asynchronous resource. All our node CI jobs are now failing with the given output:

.../node_map.cpp:484:13: error: 'Call' is deprecated [-Werror,-Wdeprecated-declarations]
        cb->Call(1, argv);
            ^

Fixes #11288.

@brunoabinader brunoabinader added build Node.js node-mapbox-gl-native labels Feb 26, 2018
@jfirebaugh
Copy link
Contributor

@brunoabinader brunoabinader added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Feb 26, 2018
@ChrisLoer
Copy link
Contributor

Alternative (temporary but also hopefully more conservative) fix at: #11321

@brunoabinader brunoabinader force-pushed the node-cb-call-asyncresource branch from c8c4208 to 941eed9 Compare February 27, 2018 17:28
@brunoabinader
Copy link
Member Author

@jfirebaugh intent was to replicate AsyncWorker's internal usage of AsyncResource without the extra encapsulation. What it seems I have missed on the initial patch was passing the map object's handle() as a param to the AsyncResource ctor.

However, we probably want to stay in the safe lane and subclass AsyncWorker to handle it instead.

@kkaefer
Copy link
Member

kkaefer commented May 23, 2018

Superseded by #11913

@kkaefer kkaefer closed this May 23, 2018
@kkaefer kkaefer deleted the node-cb-call-asyncresource branch May 23, 2018 12:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
build ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold Node.js node-mapbox-gl-native
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants