Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nckswt opened this issue Nov 15, 2021 · 11 comments
Open

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

nckswt opened this issue Nov 15, 2021 · 11 comments
Labels

Comments

@nckswt
Copy link

nckswt commented Nov 15, 2021

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 git@github.com: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 nckswt added the bug label Nov 15, 2021
@wayneparrott
Copy link
Collaborator

wayneparrott commented Nov 17, 2021

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
Copy link
Collaborator

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
Copy link
Collaborator

wayneparrott commented Nov 18, 2021 via email

@nckswt
Copy link
Author

nckswt commented Nov 18, 2021

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

@wayneparrott
Copy link
Collaborator

@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
Copy link
Author

nckswt commented Nov 19, 2021

@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  dependencies=int64-napi@1.0.2, rclnodejs@0.20.0, ref-napi@4.0.0 platform=linux arch=x64

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

@wayneparrott
Copy link
Collaborator

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
Copy link

anmilleriii commented Nov 24, 2021

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.

@nckswt
Copy link
Author

nckswt commented Dec 1, 2021

@anmilleriii All that worked perfectly to get npm run start:dev up and running. Thank you! Your reward is more questions 😁

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
Copy link

anmilleriii commented Dec 4, 2021

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.

@nckswt
Copy link
Author

nckswt commented Dec 7, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants