rclnodejs icon indicating copy to clipboard operation
rclnodejs copied to clipboard

rclnodeJs installation broken: "Could not locate the bindings file"

Open nckswt opened this issue 3 years ago • 14 comments

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:

Uncaught Error: Could not locate the bindings file. Tried:
 → /home/nick/ros2-electron-app-mvp/build/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/build/Debug/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/build/Release/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/out/Debug/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/Debug/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/out/Release/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/Release/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/build/default/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/compiled/14.16.0/linux/x64/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/addon-build/release/install-root/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/addon-build/debug/install-root/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/addon-build/default/install-root/rclnodejs.node
 → /home/nick/ros2-electron-app-mvp/lib/binding/node-v87-linux-x64/rclnodejs.node
    at bindings (bindings.js:126)
    at Object../node_modules/rclnodejs/lib/clock.js (clock.js:17)
    at __webpack_require__ (bootstrap:24)
    at fn (hot module replacement:61)
    at Object../node_modules/rclnodejs/index.js (index.js:17)
    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

This PR makes me think we should be able to load rclnodejs into the electron renderer.

  • Library Version: 0.20.1
  • ROS Version: galactic
  • Platform / OS: Ubuntu 20.04.3 LTS
  • Node version: v12.22.7

Steps To Reproduce

git clone [email protected]:nckswt/ros2-electron-app-mvp.git
npm i
npm run start:dev

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.

nckswt avatar Nov 15 '21 23:11 nckswt

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.

wayneparrott avatar Nov 17 '21 22:11 wayneparrott

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.

koonpeng avatar Nov 18 '21 10:11 koonpeng

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.

wayneparrott avatar Nov 18 '21 15:11 wayneparrott

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.

Yup! I'm using electron-builder for that

nckswt avatar Nov 18 '21 16:11 nckswt

@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.

wayneparrott avatar Nov 18 '21 22:11 wayneparrott

@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.

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  [email protected], [email protected], [email protected] platform=linux arch=x64

but I can't get further along than that. Maybe the cause is some interplay with webpack and electron builder?

nckswt avatar Nov 19 '21 22:11 nckswt

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.

wayneparrott avatar Nov 22 '21 22:11 wayneparrott

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:dev

And 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:6

I'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.

anmilleriii avatar Nov 24 '21 02:11 anmilleriii

@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?

nckswt avatar Dec 01 '21 02:12 nckswt

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.
  • By convention build/ is not generated but rather for assets to be included in the build (such as logos, etc.)
  • By convention 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)
  • Electron main AND renderer source code should be in 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/.
  • Restructuring your project to the above will require a few changes to your existing Webpack configurations which I didn't have the time to look through, but I think you will find these changes to adhere to conventions worthwhile.
  • As a design-level data point, running 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.
  • My advice to use a 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.
  • You may already be aware of this, but in production the end-user device must have ROS installed and you will need a launch script which sources ROS to run the AppImage in production (since you won't have an interactive terminal to do it). See this issue. My personal solution to this is documented here

Resources

As I probably won't be able to provide too much of a further look, hopefully these are helpful:

  • electron-react-boilerplate - Boilerplate for Electron/React. Not suggesting you use this, but you might find a few useful structural ideas from it.
  • electron-ros repository - Sample implementation of rclnodejs in Electron which covers many rclnodejs specific issues.
  • Foxglove Studio repository - Example of a very well-structured Electron app with self-managed Webpack configuration. Well beyond the scope of your requirements probably, but studying this may be helpful, as it has been for me.

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.

anmilleriii avatar Dec 04 '21 19:12 anmilleriii

No worries -- that's a wonderful and well-detailed response! Thanks so much!

nckswt avatar Dec 07 '21 02:12 nckswt