nodejs / nan

Native Abstractions for Node.js
MIT License
3.28k stars 504 forks source link

`Nan::SetAccessor()` deprecation warning with V8 10.1 and nan 2.15.0 #936

Closed mscdex closed 8 months ago

mscdex commented 2 years ago
../../../../node_modules/nan/nan.h: In function ‘void Nan::SetAccessor(v8::Local<v8::ObjectTemplate>, v8::Local<v8::String>, Nan::GetterCallback, Nan::SetterCallback, v8::Local<v8::Value>, v8::AccessControl, v8::PropertyAttribute, Nan::imp::Sig)’:
../../../../node_modules/nan/nan.h:2551:16: warning: ‘void v8::ObjectTemplate::SetAccessor(v8::Local<v8::Name>, v8::AccessorNameGetterCallback, v8::AccessorNameSetterCallback, v8::Local<v8::Value>, v8::AccessControl, v8::PropertyAttribute, v8::Local<v8::AccessorSignature>, v8::SideEffectType, v8::SideEffectType)’ is deprecated: Do signature check in accessor [-Wdeprecated-declarations]
 2551 |     , signature);
      |                ^
In file included from /home/foo/.cache/node-gyp/18.0.0/include/node/v8-function.h:15,
                 from /home/foo/.cache/node-gyp/18.0.0/include/node/v8.h:33,
                 from /home/foo/.cache/node-gyp/18.0.0/include/node/node.h:63,
                 from ../src/binding.cc:13:
/home/foo/.cache/node-gyp/18.0.0/include/node/v8-template.h:838:8: note: declared here
  838 |   void SetAccessor(
      |        ^~~~~~~~~~~
mkrufky commented 2 years ago

@mscdex Can you confirm that this is resolved in the main branch?

alejandroclaro commented 2 years ago

There is now a similar problem with Electron >= 20

see: https://github.com/electron/electron/issues/35193

danielweck commented 8 months ago

Electron29 / NodeJS 20 breakage:

../node_modules/nan/nan.h:2548:8: error: no matching member function for call to 'SetAccessor'
  tpl->SetAccessor(
  ~~~~~^~~~~~~~~~~
/Users/danielweck/Library/Caches/node-gyp/29.0.0/include/node/v8-template.h:806:8: note: candidate function not viable: no known conversion from 'v8::AccessControl' to 'PropertyAttribute' for 5th argument
  void SetAccessor(
       ^
/Users/danielweck/Library/Caches/node-gyp/29.0.0/include/node/v8-template.h:800:8: note: candidate function not viable: no known conversion from 'imp::NativeGetter' (aka 'void (*)(v8::Local<v8::Name>, const v8::PropertyCallbackInfo<v8::Value> &)') to 'AccessorGetterCallback' (aka 'void (*)(Local<String>, const PropertyCallbackInfo<Value> &)') for 2nd argument
  void SetAccessor(
mscdex commented 8 months ago

Looks like this was fixed in 2.17.0.

danielweck commented 8 months ago

Looks like this was fixed in 2.17.0.

I am getting a fatal compiler error with NAN 2.18.0 and Electron 29 / NodeJS 20, see my message above https://github.com/nodejs/nan/issues/936#issuecomment-1956558170

My current workaround is to comment out all the setAccessor functions from the postinstall script in my package.json (my project doesn't use these functions):

https://github.com/nodejs/nan/blob/e14bdcd1f72d62bca1d541b66da43130384ec213/nan.h#L2514-L2655

danielweck commented 8 months ago

PS: a while back somebody ended-up forking NAN to remove breaking function signatures:

https://github.com/electron/electron/issues/35193#issuecomment-1212384522

I personally use the official NAN NPM package and I sed into nan.h to inject comment markers /* + */.

j-stahl commented 7 months ago

I can confirm this issue persists. It's not just a warning. It throws an error now, as @danielweck mentioned. Create an empty folder and create this package.json:

{
  "name": "testproject",
  "version": "1.0.0",
  "description": "Testing electron 29.0",
  "main": "main.js",
  "scripts": {
    "postinstall": "electron-builder install-app-deps"
  },
  "author": "j-stahl",
  "license": "MIT",
  "devDependencies": {
    "@electron/notarize": "^2.3.0",
    "electron": "^29.0.1",
    "electron-builder": "^24.12.0"
  },
  "dependencies": {
    "@electron/asar": "^3.2.8",
    "electron-store": "^8.1.0",
    "electron-updater": "^6.1.8",
    "node-ssh": "^13.1.0",
    "simple-ssh": "^1.1.1"
  }
}

Run npm install and play around with the electron version. Electron < 29 will succeed.

By the way: I took more than one day to figure out what is happening here. I am not pretty familar with all those dependency stuff.

ajgassner commented 7 months ago

I can confirm, I have the same issue. Nan is not compatible with Electron 29 / Node 20. As a temporary workaround you can use following in your package.json:

  "resolutions": {
    "nan": "github:ajgassner/nan#electron-29-workaround"
  }

Please note I just commented all the SetAccessor things as a dirty quick fix as I'm not familar with the V8 API.

See https://github.com/nodejs/nan/pull/966

JasonYeMSFT commented 7 months ago

Could this be a breaking change in node-gyp's header file?

In Electron 28.2.2's v8-template.h (you can find it at _your_userdirectory/.electron-gyp/Library/Caches/node-gyp/28.2.2/include/node)

  void SetAccessor(
      Local<String> name, AccessorGetterCallback getter,
      AccessorSetterCallback setter = nullptr,
      Local<Value> data = Local<Value>(), AccessControl settings = DEFAULT,
      PropertyAttribute attribute = None,
      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
  void SetAccessor(
      Local<Name> name, AccessorNameGetterCallback getter,
      AccessorNameSetterCallback setter = nullptr,
      Local<Value> data = Local<Value>(), AccessControl settings = DEFAULT,
      PropertyAttribute attribute = None,
      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);

In Electron 29.1.0's v8-template.h

  void SetAccessor(
      Local<String> name, AccessorGetterCallback getter,
      AccessorSetterCallback setter = nullptr,
      Local<Value> data = Local<Value>(), PropertyAttribute attribute = None,
      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
  void SetAccessor(
      Local<Name> name, AccessorNameGetterCallback getter,
      AccessorNameSetterCallback setter = nullptr,
      Local<Value> data = Local<Value>(), PropertyAttribute attribute = None,
      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);

It seems PropertyAttribute and AccessControl are not compatible given the error details:

no known conversion from 'v8::AccessControl' to 'PropertyAttribute' for 5th argument

Probably nan needs to adjust its signature to match the new signature.