MadLittleMods / node-usb-detection

List USB devices in system and detect changes on them.
MIT License
371 stars 116 forks source link

Node 16 / Electron 15 prebuilds #153

Open dopry opened 2 years ago

dopry commented 2 years ago

It looks like prebuilds are being run with node 14. Node 16 is current and Electron 15 has shipped with Node 16. It would be nice to have prebuilts for the current version of node/electron.

mcous commented 2 years ago

Node v16 is ABI version 93, and I believe Electron v15 is ABI version 98. The usb-detection@4.13.0 release has prebuilt binaries for both of these ABI versions. Are these prebuilds not working for you?

The prebuild CI job happens to run on Node v14, but it is configured to build binaries for all supported ABI versions

mholtzman commented 2 years ago

@dopry I had the same issue that I think you're having. If the problem is that you're packaging your Electron app w. something like electron-builder or electron-rebuild, these both use prebuild-install which depends on node-abi v2.x. This version doesn't support Node module 98 so you either need to build prebuild-install from source using the updated node-abi version and include it in your application, or do what I did and use something like npm-force-resolutions to force prebuild-install to use an updated version.

dopry commented 2 years ago

Oh awesome thanks for the hint. We kinda gave up on it and decided to move to WebHID. If that works proves challenging I'll try updating with force resolution. Is there and upstream issue in prebuild-install I should track?

mcous commented 2 years ago

Looks like the root issue is electron/electron-rebuild#886. The fix has been merged, but a new version has not yet been cut.

Thankfully, there's nothing to be done for this on the usb-detection side to take advantage of this fix whenever it happens, the prebuilds are already in place! This package.json works for me:

{
  "name": "electron-testing",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "preinstall": "npx npm-force-resolutions",
    "postinstall": "electron-rebuild"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "resolutions": {
    "node-abi": "^3.2.0"
  },
  "dependencies": {
    "usb-detection": "^4.13.0"
  },
  "devDependencies": {
    "electron": "^15.2.0",
    "electron-rebuild": "^3.2.3"
  }
}
// npx electron index.js
const { app } = require('electron');
const usbDetection = require('usb-detection');

usbDetection.startMonitoring();

usbDetection.find((error, devices) => {
  console.log(devices, error);
});

app.once('will-quit', () => usbDetection.stopMonitoring());
mholtzman commented 2 years ago

Looks like the root issue is electron/electron-rebuild#886. The fix has been merged, but a new version has not yet been cut.

I was looking at this earlier and, while this should work for allowing electron-rebuild to recognize the correct ABI version, it ultimately still calls prebuild-install for projects like this so I don't think this will solve it.

dopry commented 2 years ago

@mcous thanks so much for your help today as well. I'm happy if you guys want to close this issue or leave it open until the upstream issues are resolved for people to find more easily.

mcous commented 2 years ago

@mholtzman yeah, I think you're right. The resolution override only works because it's overriding it in two places. That's... not ideal.

@dopry of course! I think this ticket should stay open / possibly pinned, since until upstream issues are resolved, users will be unable to install this package in Electron v15 without the above workarounds

Stringer86 commented 2 years ago

Followed the above instructions (and package.json) but still getting:

/node_modules/usb-detection/build/Release/detection.node'
was compiled against a different Node.js version using
NODE_MODULE_VERSION 89. This version of Node.js requires
NODE_MODULE_VERSION 98. Please try re-compiling or re-installing
the module (for instance, using `npm rebuild` or `npm install`).
    at process.func [as dlopen] (node:electron/js2c/asar_bundle:5:1800)
    at Object.Module._extensions..node (node:internal/modules/cjs/loader:1170:18)
    at Object.func [as .node] (node:electron/js2c/asar_bundle:5:1800)
    at Module.load (node:internal/modules/cjs/loader:982:32)
    at Module._load (node:internal/modules/cjs/loader:823:12)
    at Function.c._load (node:electron/js2c/asar_bundle:5:13331)
    at Module.require (node:internal/modules/cjs/loader:1006:19)
    at require (node:internal/modules/cjs/helpers:93:18)
"usb-detection": "^4.13.0"
"electron": "^15.2.0",
"electron-rebuild": "^3.2.3",
"electron-builder": "^22.14.5",

Any advice?

mholtzman commented 2 years ago

@dopry it looks like prebuild-install was updated to include a more current version of node-abi, so I think this issue could be fixed by updating to use prebuild-install 7.x

mholtzman commented 2 years ago

Followed the above instructions (and package.json) but still getting:

/node_modules/usb-detection/build/Release/detection.node'
was compiled against a different Node.js version using
NODE_MODULE_VERSION 89. This version of Node.js requires
NODE_MODULE_VERSION 98. Please try re-compiling or re-installing
the module (for instance, using `npm rebuild` or `npm install`).
    at process.func [as dlopen] (node:electron/js2c/asar_bundle:5:1800)
    at Object.Module._extensions..node (node:internal/modules/cjs/loader:1170:18)
    at Object.func [as .node] (node:electron/js2c/asar_bundle:5:1800)
    at Module.load (node:internal/modules/cjs/loader:982:32)
    at Module._load (node:internal/modules/cjs/loader:823:12)
    at Function.c._load (node:electron/js2c/asar_bundle:5:13331)
    at Module.require (node:internal/modules/cjs/loader:1006:19)
    at require (node:internal/modules/cjs/helpers:93:18)
"usb-detection": "^4.13.0"
"electron": "^15.2.0",
"electron-rebuild": "^3.2.3",
"electron-builder": "^22.14.5",

Any advice?

you used npm-force-resolutions and it still doesn't work? check your package-lock.json file to make sure everything is using node-abi 3.x+

sfoster1 commented 1 year ago

I'm not sure whether this is part of your configurations or an upstream thing in node-gyp or nan but v8 requires C++17 to build it properly since the v8 9.3 release (this is the commit that did it). V8 9.3 is used in Node 16.9 and later. Most compilers unfortunately don't yet default to C++17, so when your code includes node.h you eventually get errors like this during the build:

npm ERR! /home/seth/.electron-gyp/20.3.0/include/node/v8-maybe.h:106:45: error: ‘is_lvalue_reference_v’ is not a member of ‘std’; did you mean ‘is_lvalue_reference’?
    npm ERR!   106 |   template <class U, std::enable_if_t<!std::is_lvalue_reference_v<U>>*>
    npm ERR!       |                                             ^~~~~~~~~~~~~~~~~~~~~
    npm ERR!       |                                             is_lvalue_reference
    npm ERR! /home/seth/.electron-gyp/20.3.0/include/node/v8-maybe.h:106:66: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
    npm ERR!   106 |   template <class U, std::enable_if_t<!std::is_lvalue_reference_v<U>>*>
    npm ERR!       |                                                                  ^
    npm ERR! /home/seth/.electron-gyp/20.3.0/include/node/v8-maybe.h:106:67: error: template argument 1 is invalid
    npm ERR!   106 |   template <class U, std::enable_if_t<!std::is_lvalue_reference_v<U>>*>
    npm ERR!       |                                                                   ^
    npm ERR! /home/seth/.electron-gyp/20.3.0/include/node/v8-maybe.h:106:71: error: expected unqualified-id before ‘>’ token
    npm ERR!   106 |   template <class U, std::enable_if_t<!std::is_lvalue_reference_v<U>>*>
    npm ERR!       |                                                                       ^
    npm ERR! /home/seth/.electron-gyp/20.3.0/include/node/v8-maybe.h:123:43: error: ‘is_lvalue_reference_v’ is not a member of ‘std’; did you mean ‘is_lvalue_reference’?
    npm ERR!   123 | template <class T, std::enable_if_t<!std::is_lvalue_reference_v<T>>* = nullptr>
    npm ERR!       |                                           ^~~~~~~~~~~~~~~~~~~~~
    npm ERR!       |                                           is_lvalue_reference
    npm ERR! /home/seth/.electron-gyp/20.3.0/include/node/v8-maybe.h:123:64: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
    npm ERR!   123 | template <class T, std::enable_if_t<!std::is_lvalue_reference_v<T>>* = nullptr>
    npm ERR!       |                                                                ^
    npm ERR! /home/seth/.electron-gyp/20.3.0/include/node/v8-maybe.h:123:65: error: template argument 1 is invalid
    npm ERR!   123 | template <class T, std::enable_if_t<!std::is_lvalue_reference_v<T>>* = nullptr>
    npm ERR!       |                                                                 ^
    npm ERR! /home/seth/.electron-gyp/20.3.0/include/node/v8-maybe.h:123:70: error: expected unqualified-id before ‘=’ token
    npm ERR!   123 | template <class T, std::enable_if_t<!std::is_lvalue_reference_v<T>>* = nullptr>

(I know this is in the context of electron, but it's a change in v8 that would happen in any node distribution >=16.9).

If you run CPPFLAGS="-std=c++17" (npm|yarn) install, or if you downgrade to node 16.8, the build succeeds fine.

sfoster1 commented 1 year ago

Also fyi: https://github.com/electron/electron/issues/35193 affects this module too - anything that uses nan and targets recent node