How to use multiple .node files?

[todbot]

Hi, I'm the current maintainer of node-hid. It supports two different back-end native libraries on Linux: libusb or hidraw. The bindings loading currently looks like:

            if( !driverType || driverType === 'hidraw' ) {
                binding = require('bindings')('HID_hidraw.node');
            } else {
                binding = require('bindings')('HID.node');
            }
        }

How do I accomplish something similar using prebuildify and node-gyp-build?

I have a test repo that generates two simple .node libraries here: https://github.com/todbot/napi-demo
I can see that node-gyp is correctly building both:

$ ls -1 build/Release/*node
build/Release/hello1.node*
build/Release/hello2.node*

but prebuildify is only taking hello1.node and renaming it to node.napi.node, leaving hello2.node behind. And I don't see now in node-gyp-build I can specify which .node.

[mafintosh]

We only support one .node file atm, but we have a tagging api we could improve to support this.
If you wanted support this currently you'd have to publish two deps each with the relevant .node file.

[mafintosh]

Actually we could just preserve the name now. The reason we call it "node" is legacy (we used to make seperate builds for electron pre-napi).

So we could just preserve the name of the files (anything before the first . char) and then have node-gyp build support a name ie

const binding1 = require('node-gyp-build')(__dirname, { name: 'hello1' })
const binding2 = require('node-gyp-build')(__dirname, { name: 'hello2' })

[mafintosh]

If you wanna help with o/ let me know, otherwise we'll get to it at some point :)

[vweevers]

If we're dropping support of non-napi we can also remove the napi and abi tags

[mafintosh]

+1 to dropping napi, I think we might wanna keep the abi one incase you wanted to target a specific abi for a bugfix for only that version, ie you have your general one and then one for a specific abi

[vweevers]

What should the default name be?

1. If name is not provided, return the first matching *.node file, same as today
2. Fallback to package.json name. Assuming we can normalize it to match NODE_GYP_MODULE_NAME, e.g. foo-bar => foo_bar

I ask because option 2 could enable searching a flat directory that contains multiple addons, produced by a bundler like zeit/pkg (prebuild/node-gyp-build#27) or webpack (prebuild/node-gyp-build#22). Which kinda requires that all consumers of node-gyp-build support that.

[Julusian]

What should the default name be?

1. If name is not provided, return the first matching *.node file, same as today
2. Fallback to package.json name. Assuming we can normalize it to match NODE_GYP_MODULE_NAME, e.g. foo-bar => foo_bar

I ask because option 2 could enable searching a flat directory that contains multiple addons, produced by a bundler like zeit/pkg (prebuild/node-gyp-build#27) or webpack (prebuild/node-gyp-build#22). Which kinda requires that all consumers of node-gyp-build support that.

If you want to fix support for webpack then I dont think you can safely rely on either of those fallbacks.

Once it is webpacked, in the unlikely scenario that you do know and manage to load the correct package.json, then someone has configured webpack to mangle __dirname differently from the default, and has prepared the files on disk sufficiently meaning the prebuilds will be in the usual place too. But if you don't explicitly set that property in webpack, you stand no chance of finding the correct package.json

If you make an important parameter like this optional, then you are prioritising convenience of a library author over users of that library. It would take a library author a couple of minutes to adjust their call to node-gyp-build when updating that dependency, instead of making a user convince the library author to make a change that they may not understand the need for. That will be the case in some projects anyway, but it will be an easier sell of "I need you to update node-gyp-build so that your library works with webpack" with the docs for node-gyp-build confirming that the major version bump and api change was to fix support for webpack

[vweevers]

I don't agree with that prioritization though. Convenience for library authors - and us avoiding breaking changes that create work for them if they possibly don't care about webpack at all - is quite important. That work also includes understanding the change which takes more than a couple of minutes.

Using webpack is the user's choice, and makes it their responsibility to make it work, considering that webpack is not the only bundler, and that webpack made the unfortunate choice of dropping compatibility with node modules.

That said, of course compatibility with webpack is a welcome feature, and if done right, would ultimately also save library authors time. We can get there incrementally, starting with a name option that library authors can opt-in to. This also helps to find any unforeseen issues.

[Julusian]

Convenience for library authors - and us avoiding breaking changes that create work for them if they possibly don't care about webpack at all - is quite important.

I agree that minimising unnecessary work for them is important (I do neglect updating libraries which do semver major jumps too often), but that doesnt mean it shouldnt be done.

That work also includes understanding the change which takes more than a couple of minutes.

Does it? Both prebuidify and node-gyp-build would complain at the lack of name when being used, and can give a hint at what it should be set to. Or they would check the docs and see the one line justification and description of the new property.

Using webpack is the user's choice, and makes it their responsibility to make it work,

Except that by making it the users responsibility you are leaving them to choose between some painful choices:

1. Hope the library maintainer will be receptive to making the required change in a reasonable timeframe. In my experience it is rare to get a fix released within a couple of days (it normally takes weeks), so other options will likely be explored while waiting.
2. Dont use that library
3. Change webpack configuration and hope other dependencies dont break, and probably updating any uses of __dirname in your own code to do something different when in a webpack bundle
4. Fork the library to set the field, generate prebuilds and publish to npm

considering that webpack is not the only bundler, and that webpack made the unfortunate choice of dropping compatibility with node modules.

From what I gather it does affect other bundlers too, and it makes sense to. Its not that webpack doesnt support node_modules, its that __dirname is rather flawed when in a bundle. This is only a problem in webpack when you bundle the node_modules, if they stay like normal on disk then there is no problem.
Webpack by default makes it /, making everything relative to the generated bundle file. Which confuses node-gyp-build which is looking for path.join(__dirname, 'package.json').
Alternatively, you could not mangle it and end up with __dirname being ../../node_modules/something/node_modules/another/node_modules/lib-with-native. Yeah, this can work, but something needs to generate that directory structure. webpack has no idea what files you will be loading through the dynamic require calls, so that means some other tooling is required. The ../.. prefix there was intentional, as that will happen in a mono-repo and so will make a real mess for deployment.

We can get there incrementally, starting with a name option that library authors can opt-in to. This also helps to find any unforeseen issues.

I am fine with this approach, but do think it needs to be a required option in a future release

[vweevers]

Its not that webpack doesnt support node_modules, its that __dirname is rather flawed when in a bundle.

A bundler that targets Node.js should make sure that __dirname works the same as when the code is not bundled. Then at least the expected location of the addon is predictable. If the bundler primarily targets browsers, then it is perfectly reasonable to expect users to correctly configure that bundler. If it then complies with Node.js behavior, there is no reason to believe it would break other dependencies (that also target Node.js). But I do hear you that the expected location of the addon is not always ideal.