Open nckswt opened 2 years ago
wayneparrott
commented
2 years ago I was hoping someone with more electron experience than me would pick up this issue. I'll give it a go tomorrow and see what I learn. @nckswt Thx for the mvp. Please share any new info you may have on the matter.
koonpeng
commented
2 years ago I haven't used electron in a while, so some of these information might be outdated.
Normally the binding files are built when you install rclnodejs, this is done by node-gyp which is bundled with every node installation. These node bindings are normally not compatible with electron, for electron, there is a tool which helps build the bindings (iirc its called electron-rebuild) in a way that is compatible.
wayneparrott
commented
2 years ago Thank you for your quick response. I'll give it a try asap. Out of curiosity are you using rclnodejs for anything interesting these days? Wayne
On Thu, Nov 18, 2021 at 4:14 AM Teo Koon Peng @.***> wrote:
I haven't used electron in a while, so some of these information might be outdated.
Normally the binding files are built when you install rclnodejs, this is done by node-gyp which is bundled with every node installation. These node bindings are normally not compatible with electron, for electron, there is a tool which helps build the bindings (iirc its called electron-rebuild) in a way that is compatible.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/RobotWebTools/rclnodejs/issues/820#issuecomment-972725009, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKUP4WQTDICGPHNXLS2CMLUMTG2BANCNFSM5IC5DZYQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
nckswt
commented
2 years ago I haven't used electron in a while, so some of these information might be outdated.
Normally the binding files are built when you install
rclnodejs, this is done bynode-gypwhich is bundled with every node installation. These node bindings are normally not compatible with electron, for electron, there is a tool which helps build the bindings (iirc its calledelectron-rebuild) in a way that is compatible.
Yup! I'm using electron-builder for that
wayneparrott
commented
2 years ago @nckswt I pulled and built your mvp example without replicating the missing bindings issue. But it did not run. So being a noob I stepped back and created a simpleton example with no bundler to get something working. https://github.com/ros2jsguy/rclnodejs-electron-example After installing dependencies I rebuilt rclnodejs using the npx electron-rebuild cmd. This very basic example runs in my ubuntu 20 env with node 16 and ros2 galactic.
Next I will extend it to use webpack similar to your example. Note this example is based on Electron v16 & the rclnodejs#node-16 branch. We are working to publish this branch asap.
nckswt
commented
2 years ago @nckswt I pulled and built your mvp example without replicating the
missing bindingsissue. But it did not run. So being a noob I stepped back and created a simpleton example with no bundler to get something working. https://github.com/ros2jsguy/rclnodejs-electron-example After installing dependencies I rebuilt rclnodejs using thenpx electron-rebuildcmd. This very basic example runs in my ubuntu 20 env with node 16 and ros2 galactic.Next I will extend it to use webpack similar to your example. Note this example is based on Electron v16 & the rclnodejs#node-16 branch. We are working to publish this branch asap.
Ahh, I was hoping there'd be a node 16 solution! Didn't realize this branch existed. I tried including it as
"rclnodejs": "git://github.com/RobotWebTools/rclnodejs.git#nodejs-16",with electron 16 and still ran into the same issues though (even after rm -rf package-lock.json node_modules). I suspect the issue's coming from when electron-builder executes the following:
• rebuilding native dependencies dependencies=int64-napi@1.0.2, rclnodejs@0.20.0, ref-napi@4.0.0 platform=linux arch=x64but I can't get further along than that. Maybe the cause is some interplay with webpack and electron builder?
wayneparrott
commented
2 years ago Yes I believe it is a webpack matter but I have not dug in deeper. I successfully impl'ed a small port of my basic electron-rclnodejs app to use the electron-builder "electron-boilerplate" template https://github.com/szwacz/electron-boilerplate and also preliminary results using electron-forge seem to be working in dev mode. My dev env is linux.
Did you use a boilerplate project for your mvp example? Once we figure out what's goofed I plan to doc up the process and maybe do a short step-by-step video.
anmilleriii
commented
2 years ago Hi friends, I have encountered similar build issues using Webpack and Electron previously, and thought I would take a look.
I cloned your example repo and ran the following:
source /opt/ros/foxy/setup.bash
npm install
npm run start:devAnd was met with the missing rclnodejs bindings error similar to as described above. As @koonpeng mentions, the rclnodejs bindings are not immediately ready for Electron. We need to tell Webpack not to bundle rclnodejs normally, but keep as an unbundled external instead.
Therefore I added the following lines to both the webpack.dev.config.js and webpack.prd.config.js:
module.exports = [{
...
target: 'electron-renderer',
externals: {
rclnodejs: 'rclnodejs'
},
...
]This removed the bindings error, but instead gave:
Uncaught ReferenceError: rclnodejs is not defined
at Object.rclnodejs (external var "rclnodejs":1)
at __webpack_require__ (bootstrap:24)
at fn (hot module replacement:61)
at Object../src/index.js (index.js:11)
at __webpack_require__ (bootstrap:24)
at startup:6
at startup:6I'm no expert with nodeIntegration and contextIsolation, and I might have expected not to have to expose rclnodejs through a window global, but for whatever reason that is what worked. Adding a preload script to the mainWindow:
webPreferences: {
preload: path.join(__dirname, 'preload.js'),
...
}And the preload.js file at public/preload.js:
window.rclnodejs = require('rclnodejs')Running npm run start:dev again (make sure to source ROS) worked as expected.
nckswt
commented
2 years ago @anmilleriii All that worked perfectly to get npm run start:dev up and running. Thank you! Your reward is more questions :grin:
The second (and last) step to this problem is to get npm run start:dist up and running. This uses electron-builder to generate a mvpApp.AppImage bundled executable that should be run without NodeJS installed.
So, taking the rclnodejs external approach means I'd need to find a way to include rclnodejs in the AppImage build as well. This is possible by adding the following extraResources configuration to my package.json:
"build": {
...
"extraResources": [{
"from": "./node_modules/",
"to": "node_modules",
"filter": [
"rclnodejs/**/*"
]}
],That puts rclnodejs under the distributable package. But, there's a catch -- it didn't bundle any dependencies (or dependencies of dependencies), so instead you have to do something like:
"build": {
...
"extraResources": [{
"from": "./node_modules/",
"to": "node_modules",
"filter": ["**/*"]
}],This works, but it bloats the AppImage pretty dramatically (from 80MB to 175MB).
Note that you can run npx npm-remote-ls rclnodejs --flatten --optional false --development false to find the complete dependency tree of rclnodejs and filter out those packages, but: it's about 60 packages long; is incredibly brittle; doesn't seem to cover all deps so doesn't work anyway.
I've updated my mvp example to use this configuration. Wondering if there's a more elegant solution I can't think of? Or if that's the expected workaround?
anmilleriii
commented
2 years ago I apologize but I was not able to spend the time to debug an ENOENT for ../app.asar/build/preload.js error. This error is related to the webpack.prd.config.js, package.json, and the location of preload.js.
While I wanted to quickly solve the npm run start:dist, instead I've provided some high-level comments which provide context on whether a preload.js script should be used at all, among other items which I hope you find helpful.
Comments
extraResources can be helpful for keeping some production files writeable (e.g., a database), but as you have identified, it is not appropriate to include unbundled node_modules/. I personally have not encountered an issue including rclnodejs in the AppImage as well as in development - did you trace the error directly to rclnodejs being missing or was it the preload.js error that I alluded to above (which would make rclnodejs undefined in production) that stopped you? Also see this issue.build/ is not generated but rather for assets to be included in the build (such as logos, etc.)public/ is solely for the static files through which the rest of the application is mounted (i.e., usually just an index.html file and favicon.icon)src/. While the structure is up to you, often people use src/main for the main process file (i.e., main.js) and src/renderer for the renderer process - i.e., your React app source code. The big idea is simply that source code doesn't live in public/.rclnodejs in the view (e.g., React app) renderer process is not necessary to distribute it with Electron. You can have multiple "renderer" (or more aptly, "worker") Electron proccesses which run in Node and communicate with the view over WebSocket connection, ipcRenderer, or other communication bridge. Our team found this encapsulation of rclnodejs to be simpler to work with, since it decouples rclnodejs and ROS from your view layer. See electron-ros below for more details.preload.js isn't a best practice with nodeIntegration enabled, since the purpose of a preload script is to use Node modules in the browser with nodeIntegration disabled. Was just a quick fix to run the project as-is.Resources
As I probably won't be able to provide too much of a further look, hopefully these are helpful:
rclnodejs in Electron which covers many rclnodejs specific issues.Lastly @wayneparrott if @nckswt is okay with it, I might suggest closing this issue or at least removing the bug label, as the original bindings file issue is more of a limitation of rclnodejs integration with Webpack than a bug with rclnodejs itself. Once again sorry I couldn't immediately fix the npm run start:dist issue.
nckswt
commented
2 years ago No worries -- that's a wonderful and well-detailed response! Thanks so much!
dineshHarikrishnan
commented
2 months ago When Installing rclnodejs library, I am getting this
my nodeJS version - 20.11.0
I dont want to degrade my nodeJs version. but it needs to be installed.
npm i rclnodejs
npm WARN deprecated har-validator@5.1.5: this library is no longer supported
npm WARN deprecated npmlog@4.1.2: This package is no longer supported.
npm WARN deprecated are-we-there-yet@1.1.7: This package is no longer supported.
npm WARN deprecated uuid@3.4.0: Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details.
npm WARN deprecated gauge@2.7.4: This package is no longer supported.
npm WARN deprecated request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142
npm ERR! code 1
npm ERR! path /home/dinesh/boson_webapp/server/node_modules/rclnodejs
npm ERR! command failed
npm ERR! command sh -c npm run rebuild
npm ERR! > rclnodejs@0.27.1 rebuild
npm ERR! > npm run clean && node-gyp -j 16 rebuild
npm ERR!
npm ERR!
npm ERR! > rclnodejs@0.27.1 clean
npm ERR! > node-gyp clean && rimraf ./generated
npm ERR! gyp info it worked if it ends with ok
npm ERR! gyp info using node-gyp@10.0.1
npm ERR! gyp info using node@20.11.0 | linux | x64
npm ERR! gyp info ok
npm ERR! gyp info it worked if it ends with ok
npm ERR! gyp info using node-gyp@10.0.1
npm ERR! gyp info using node@20.11.0 | linux | x64
npm ERR! gyp info find Python using Python version 3.8.10 found at "/usr/bin/python3"
npm ERR! gyp info spawn /usr/bin/python3
npm ERR! gyp info spawn args [
npm ERR! gyp info spawn args '/usr/local/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
npm ERR! gyp info spawn args 'binding.gyp',
npm ERR! gyp info spawn args '-f',
npm ERR! gyp info spawn args 'make',
npm ERR! gyp info spawn args '-I',
npm ERR! gyp info spawn args '/home/dinesh/boson_webapp/server/node_modules/rclnodejs/build/config.gypi',
npm ERR! gyp info spawn args '-I',
npm ERR! gyp info spawn args '/usr/local/lib/node_modules/npm/node_modules/node-gyp/addon.gypi',
npm ERR! gyp info spawn args '-I',
npm ERR! gyp info spawn args '/home/dinesh/.cache/node-gyp/20.11.0/include/node/common.gypi',
npm ERR! gyp info spawn args '-Dlibrary=shared_library',
npm ERR! gyp info spawn args '-Dvisibility=default',
npm ERR! gyp info spawn args '-Dnode_root_dir=/home/dinesh/.cache/node-gyp/20.11.0',
npm ERR! gyp info spawn args '-Dnode_gyp_dir=/usr/local/lib/node_modules/npm/node_modules/node-gyp',
npm ERR! gyp info spawn args '-Dnode_lib_file=/home/dinesh/.cache/node-gyp/20.11.0/<(target_arch)/node.lib',
npm ERR! gyp info spawn args '-Dmodule_root_dir=/home/dinesh/boson_webapp/server/node_modules/rclnodejs',
npm ERR! gyp info spawn args '-Dnode_engine=v8',
npm ERR! gyp info spawn args '--depth=.',
npm ERR! gyp info spawn args '--no-parallel',
npm ERR! gyp info spawn args '--generator-output',
npm ERR! gyp info spawn args 'build',
npm ERR! gyp info spawn args '-Goutput_dir=.'
npm ERR! gyp info spawn args ]
npm ERR! [eval]:1
npm ERR! console.log(process.env.AMENT_PREFIX_PATH.replace(/:/g, '/include/ ') + '/include/')
npm ERR! ^
npm ERR!
npm ERR! TypeError: Cannot read properties of undefined (reading 'replace')
npm ERR! at [eval]:1:43
npm ERR! at runScriptInThisContext (node:internal/vm:144:10)
npm ERR! at node:internal/process/execution:109:14
npm ERR! at [eval]-wrapper:6:24
npm ERR! at runScript (node:internal/process/execution:92:62)
npm ERR! at evalScript (node:internal/process/execution:123:10)
npm ERR! at node:internal/main/eval_string:51:3
npm ERR!
npm ERR! Node.js v20.11.0
npm ERR! gyp: Call to 'node -e "console.log(process.env.AMENT_PREFIX_PATH.replace(/:/g, '/include/ ') + '/include/')"' returned exit status 1 while in binding.gyp. while trying to load binding.gyp
npm ERR! gyp ERR! configure error
npm ERR! gyp ERR! stack Error: gyp failed with exit code: 1
npm ERR! gyp ERR! stack at ChildProcess.
npm ERR! A complete log of this run can be found in: /home/dinesh/.npm/_logs/2024-07-29T06_45_15_642Z-debug-0.log
chfritz
commented
2 months ago @dineshHarikrishnan it seems that your process.env.AMENT_PREFIX_PATH is not defined. Did you source your ros2 env before trying to install rclnodejs?
minggangw
commented
1 week ago Please check with rclnodejs Electron demo just added https://github.com/RobotWebTools/rclnodejs/tree/develop/electron_demo
Description We've been transitioning an electron app that uses roslibjs over to a ros2 interface. But, currently pulling our hair out trying to get things playing nicely.
Here's an MVP application (https://github.com/nckswt/ros2-electron-app-mvp) to recreate the issue. The main issue we're facing is this error:
This PR makes me think we should be able to load
rclnodejsinto the electron renderer.0.20.1galacticUbuntu 20.04.3 LTSv12.22.7Steps To Reproduce
Expected Behavior
I'd expect the node to run without crashing.
Actual Behavior See error above.
Wondering if there's something I need to do to generate the bindings file? Or if there's an incompatibility with my NodeJS setup somehow? Or if it's even an upstream issue?
Please let me know if there's more info I can provide for debugging.