Repository navigation
APIs removed in V8 7.0 and native addons #23122
Description
Activity
- added this to the milestone
on Sep 27, 2018 wasn't our big plan always to shim 68 and 69 for 10.x and them push 70 to 11.x?
/cc @nodejs/v8-update
Most of these APIs (all widely used ones, at least) do currently not print deprecation warnings when building with Node 10.
What if we float a patch that changes that? Doesn't break ABI but should result in a flurry of bug reports against add-ons that need upgrading.
I highly doubt that that is going to restore CITGM results in time for Node 11
If 2018-10-23 is still the target date then I agree. Floating shims for a release cycle doesn't trouble me.
The V8 commit in question is v8/v8@5acf205, btw. I don’t know if there are others that are relevant.
Most of these APIs (all widely used ones, at least) do currently not print deprecation warnings when building with Node 10.
What if we float a patch that changes that? Doesn't break ABI but should result in a flurry of bug reports against add-ons that need upgrading.
That seems like a good idea. Looking into it, it’s doable but it’s not trivial because some of the replacement APIs don’t even exist in Node 10.
BTW: 7.0 is not stable yet, we could petition for reverting of v8/v8@5acf205.
@nodejs/v8 ^^^
I just took a look. It seems the change in question is a mix of removing APIs that are marked
V8_DEPRECATEDandV8_DEPRECATE_SOON. We are conforming to the policy of marking a deprecated API for at least one version before removing. For the latter, we unfortunately are not.@danelphick has offered to partially revert this change to address the latter. I.e. instead of removing, we would move them to V8_DEPRECATED. We will still remove APIs that were already marked V8_DEPRECATED though.
However, the underlying issue here is that V8 version bumps happen a lot more often than Node.js. If V8 was to accommodate to Node.js' deprecation policy, we would need a whole year to remove an API. We did that previously with the legacy debugging protocol, but I don't think this is something we can do in general.
I think we need an actual discussion on this underlying issue, going forward.
The motivation behind this particular change is to share objects between isolates in order to save memory. That means that where we derived the isolate from the object via
GetIsolate(), we no longer can do that and require the embedder to pass the isolate explicitly.Reacted by Refael AckermannI think in this particular change it's possible to shim the removed API with a floating patch.
However, this may not solve similar issues in the future.
Reacted by Anna HenningsenWould it be possible to restate Node.js' deprecation policy towards native modules to exclude V8's API? Rationale:
- It's easier for us :)
- Node.js doesn't own V8's API.
- NAPI is a viable alternative.
- As module author, you could track node's master branch to detect API issues early.
Reacted by snek, Ruben Bridgewater and Yahor SiarheyenkaReacted by Yahor SiarheyenkaReacted by Yahor SiarheyenkaWe have some text in the collaborator guide: https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#breaking-changes-and-deprecations
Note that errors thrown, along with behaviors and APIs implemented by dependencies of Node.js (e.g. those originating from V8) are generally not under the control of Node.js and therefore are not directly subject to this policy. However, care should still be taken when landing updates to dependencies when it is known or expected that breaking changes to error handling may have been made. Additional CI testing may be required.
39 remaining items
- removed this from the milestone
on Oct 17, 2018 Should this remain open? If so, is there anything specific that is actionable at this time?
- added a commit that references this issue
on May 17, 2019 - added 2 commits that reference this issue
on Aug 7, 2019 There was no activity for over a year and we are now at V8 8.0. I guess this is resolved and I am closing this. Please reopen in case there is more left to do.
- added a commit that references this issue
on Jan 22, 2021 - added 2 commits that reference this issue
on Feb 1, 2021 - added a commit that references this issue
on Feb 11, 2021 - added a commit that references this issue
on Mar 30, 2021 - added a commit that references this issue
on Apr 23, 2021
Currently, CITGM failures are dominated by addons failing because of APIs that were removed in V8 7.0 (Example run).
Most of the removed functions are simple wrappers that take now-required arguments from other sources, for example, the now-removed
value->ToString()was essentially just a shorthand forvalue->ToString(Isolate::GetCurrent()->GetCurrentContext()).FromMaybe(Local<String>()).Most of these APIs (all widely used ones, at least) do currently not print deprecation warnings when building with Node 10.
So, the question is: What, if anything, do we do about this?
We have the option of maintaining the deprecated APIs ourselves for a while, but I’m not sure how people feel about that.
We can also do ecosystem outreach on our own, but I highly doubt that that is going to restore CITGM results in time for Node 11, and given the large number of addons that are affected by this, we definitely can’t address all cases where this is an issue by ourselves.