jasongin / noble-uwp

Noble (Node.js Bluetooth LE) with Windows 10 UWP bindings
MIT License
83 stars 45 forks source link

Consider rewriting without NodeRT #4

Open jasongin opened 7 years ago

jasongin commented 7 years ago

I'm somewhat conflicted about this project's usage of NodeRT, that enables calling WinRT APIs from Node.js JavaScript.

I originally started using NodeRT because it allowed me to easily port an earlier (unpublished) version of this project that had used Node-Chakra WinRT projections. At least, it was supposed to be easy. In practice, I've encountered a number of issues/drawbacks with the use of NodeRT:

  1. I had to manually edit the Node-RT generated code in one place to fix a compilation error.
  2. NodeRT's WinRT to JS projections aren't as good as Chakra's. With NodeRT, extra JavaScript code is required to adapt many API projections, for example to convert a WinRT IVectorView to a JS array. This has cost a lot of extra development and debugging time.
  3. The build is 7x slower than it needs to be due to needing to build NodeRT adapter assemblies for 7 namespaces (where most of the code in each is not actually used).

As an alternative, this package could be written using a more ordinary Node.js native module, with C++ code calling WinRT APIs directly and implementing most of the logic, and a thin JavaScript adapter for the noble bindings.

But since it's working as it is, I don't have the time or motivation to rewrite it all now. Maybe someday there will be a better way of calling WinRT APIs from Node.js JavaScript.

nadavbar commented 7 years ago

@jasongin - I just came across this project. Really cool! Regarding issue number 1 - against which windows SDK are you working? I didn't see this error when I pushed the gatt NodeRT module to npm back in October.

Regarding 3 - The solution will definitely be using some sort of a dynamic projection mechanism like Chakra uses.. BTW, as a workaround you can use pre-built binaries (something like node-pre-gyp)

Either way - feel free to open this issue in the main NodeRT repo..

jasongin commented 7 years ago

Hi @nadavbar!

This project requires new APIs in the Windows 10 Creators Update, so for that reason I wasn't able to use the pre-published packages under the nodert-win10 npm scope. Now that the final version of the SDK has been released (10.0.15063.0), I assume you will publish new revisions of the packages?

I had considered using pre-built binaries, but I've never worked with node-pre-gyp before and just haven't put in the time to learn how to set that up. (And there wasn't much point with the pre-release SDK while the version kept changing.)

I will open an issue about the compilation error in the generated code in the NodeRT repo.

jasongin commented 7 years ago

The solution will definitely be using some sort of a dynamic projection mechanism like Chakra uses.

Yes. I actually work with the Chakra team, including some of the people who wrote the Chakra WinRT projection code. We've discussed the possibility of bringing that capability to Node-ChakraCore, likely as an addon module since that functionality doesn't belong in the cross-platform core. But unfortunately it hasn't been a priority for them.

VinciShark commented 7 years ago

Chakra not support Electron.atom. That's why we choose noble instead of writing code based on Chakra.

Janneman84 commented 7 years ago

The dll approach would probably save lots of space too. My noble-uwp folder is currently 262MB(!), the regular noble is more like 1. Is there a way to reduce this at all? Like files you don't need once it's built?

Too bad making native node plugins isn't my kind of expertise so I can't really help.

jasongin commented 7 years ago

Is there a way to reduce this at all? Like files you don't need once it's built?

You can delete the build subdirectory from each of the UWP namespace directories. That will clean up a lot of very large intermediate files. The actual build output binaries to be kept are copied to separate binding subdirectories.

ghost commented 6 years ago

How is chakra's n-api support ? Seems like that'd be the way to go now that n-api is stable.

jasongin commented 6 years ago

Node-ChackraCore supports N-API, and N-API is officially supported by LTS versions of node (no longer experimental). So yes if noble-uwp were to be rewritten as an ordinary native module (without NodeRT) then it would make sense to use N-API. That will make it a tiny bit easier, but it will still be a lot of work.

geovie commented 6 years ago

I started a rewrite using C++/WinRT and node-addon-api at https://github.com/Timeular/noble-winrt

jasongin commented 5 years ago

It appears NodeRT isn't being very actively maintained any longer, and it hasn't been updated to generate code that works with Node 12. So that's another reason the dependency on NodeRT is a problem.