nodejs / nan

Native Abstractions for Node.js
MIT License
3.27k stars 501 forks source link

chore: use new constructor for ScriptOrigin when >= 17.x #960

Closed codebytere closed 2 months ago

codebytere commented 10 months ago

Refs https://chromium-review.googlesource.com/c/v8/v8/+/3395880.

Chromium did an advance deprecation removal of the ScriptOrigin ctor without an Isolate.

This allows Electron to remove a patch

kkoopa commented 10 months ago

Why was the check originally for Node 18? I am AFK for about a week and cannot look into it until later.

On November 1, 2023 9:26:18 PM GMT+01:00, Shelley Vohr @.***> wrote:

Refs https://chromium-review.googlesource.com/c/v8/v8/+/3395880.

Chromium did an advance deprecation removal of the ScriptOrigin ctor without an Isolate.

This allows Electron to remove a patch You can view, comment on, or merge this pull request online at:

https://github.com/nodejs/nan/pull/960

-- Commit Summary --

  • chore: use new constructor for ScriptOrigin when >= 17.x

-- File Changes --

M test/cpp/news.cpp (4)

-- Patch Links --

https://github.com/nodejs/nan/pull/960.patch https://github.com/nodejs/nan/pull/960.diff

codebytere commented 10 months ago

@kkoopa that's when the new constructor was initially slated to be removed, but they moved up the date after all uses in Chromium got removed it seems.

codebytere commented 8 months ago

@kkoopa I'd still love a look at this when you can!

codebytere commented 2 months ago

They've now removed this ctor entirely: https://chromium-review.googlesource.com/c/v8/v8/+/5539888

So we'll likely need to pend this until Node.js contains the version of V8 that removes it as well.