fix: propagate async context#199
Conversation
Start with Nan 2.9.0 certain varaiants of Nan::Callback::Call are deprecated. Overloads that preserve the async context are preferred instead. We associate a Nan::AsyncResource corresponding to a FSEvents instance. This captures the async execution context which is restored when the async callback is called. For more information see https://github.com/nodejs/nan/blob/HEAD/doc/node_misc.md#nanasyncresource.
|
The v0.12 build error is ibmdb/node-ibm_db#276 (comment). I guess today is the day we say goodbye to v0.12. Ali, can you apply this patch? diff --git a/.travis.yml b/.travis.yml
index f98b0a6..1fe7d11 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -10,7 +10,6 @@ env:
- NODE_VERSION="v6"
- NODE_VERSION="v5"
- NODE_VERSION="v4"
- - NODE_VERSION="v0.12"
- NODE_VERSION="v0.10"
before_install:@es128 How do you feel about dropping support for v0.10 as well? |
With new enough compilers, V8 headers corresponding to 0.12 no longer compile. For more details see https://github.com/nodejs/nan#compiling-against-nodejs-012-on-osx
|
Thanks for reviewing! Added commit to drop builds against Node 0.12. |
|
LMK if anything more is needed from my side (e.g. dropping 0.10 as well). |
|
Seems like we need to bump major to publish this regardless, but if 0.10 will still work it seems like we should leave it alone to reduce impact on users. For the population that's stuck on old versions, probably a lot more of them on 0.10 than 0.12. Other thoughts? |
|
@es128: I am not sure this deserves to be semver major. The 0.12 builds on OS X would already be broken if you have a new enough compiler; or they would be fine if you have a old enough compiler. IOW, this change doesn't cause the breaking change, it merely drops the platforms that are known not to be compilable in the CI anymore from the CI. |
|
Since we have been providing pre-built binaries, not doing that anymore will be a breaking change for any users on 0.12. |
|
Ah, I see. I guess what you are saying is that there is no means at present to release a fix on the current semver major line; so you need to bump major independently and bundle this change in. |
|
I'm a little torn. v0.12 has been out of support for more than a year now and the ecosystem has largely moved on. Users stuck on <= v0.12 have presumably learned to pin their dependencies by now. I don't wholly disagree with the premise that we should bump major when we drop support for v0.12 but that means downstream users won't get updates until they update their package.json. And since inertia is a powerful force, that might be a while. |
|
Sorry for the long delay. So it's a bit of deviation from strict semver, but the pros would seem to outweigh the cons. Let's bring this in without bumping major. |
|
My latest thinking after a bit of further discussion in #201 (comment) is to bump major here, but then just bump patch in chokidar to get the fix propagated easily. Pretty much all usage of fsevents goes through chokidar. Feel free to chime in with alternate opinions. |
Start with Nan 2.9.0 certain variants of
Nan::Callback::Callaredeprecated. Overloads that preserve the async context are preferred
instead.
We associate a
Nan::AsyncResourcecorresponding to a FSEvents instance.This captures the async execution context which is restored when the
async callback is called.
For more information see https://github.com/nodejs/nan/blob/HEAD/doc/node_misc.md#nanasyncresource.
EDIT: this fixes the compile warnings that are presently displayed: