Repercussions of node-pre-gyp's find method

[alex-sherwin]

Out of curiosity, is this truly necessary?

node-libcurl/lib/Curl.ts

Line 45 in 8d9b62d

const bindingPath = binary.find(

While in practice it works, it brings some baggage with it that I haven't found any way to work around.

There's a few things about this that are problematic

Importing default export of node-pre-gyp

This import:

node-libcurl/lib/Curl.ts

Line 15 in 8d9b62d

import binary from 'node-pre-gyp'

Is problematic because it forces a bundler like Webpack to process all the require statements in the top level node-pre-gyp module, which includes (among other things) aws-sdk (which then basically includes the whole world) for the publish/unpublish support it has for S3.

Obviously aws-sdk is not necessary at runtime, but there's no real way to work around this that I'm aware of or could figure out

I'm wondering if it would be possible to just import the find method module more directly instead of from the root node-pre-gyp, something like this maybe:

import { find } from "node-pre-gyp/lib/pre-binding";

Which would then trigger require statements directly related to that function

The usage of a dynamic require(..) call for locating the binding

This require statement to load the native binding:

node-libcurl/lib/Curl.ts

Line 49 in 8d9b62d

const bindings: NodeLibcurlNativeBinding = require(bindingPath)

Is problematic because it's an expression, which bundlers like Webpack have a really hard time dealing with.

As it stands right now, you can't use the Webpack externals mechanism to target this require statement because it's an expression, so it's only possible to mark the entire node-libcurl package as external, but then this means that all dependencies and transitive dependencies of node-libcurl (which also includes aws-sdk as mentioned above) also become externalized in the resultant Webpack output

Mitigation

I'm currently maintaining my own fork of this repo where I simply am not using node-pre-gyp's find method at all and changed the import for the native module to

const bindings: NodeLibcurlNativeBinding = require('../lib/binding/node_libcurl.node')

As far as I can tell, there's no reason this wouldn't work out of the box for non-bundled consumers of node-libcurl

And it has the benefit for Webpack or other bundlers that the automatic behavior of node-loader for native modules can detect the native require statement and package everything up nicely with no special effort required.

If you make this change, a consumer bundling it into an Electron app will be able to have 100% of the JavaScript sources bundled by webpack and end up with a single native_modules file for the node_libcurl.node file and it all "just works"

Real-world example

If you look at the built Electron app that, for example, Insomnia produces. You will see there's a Webpack bundle for most stuff, but then a huge node_modules of unbundled dependencies.

This is because there's no practical way to fix the issue with the way (that I'm aware of) node-libcurl can be consumed without having to drop the entire module down to an external, and due to the aws-sdk issue mentioned above, it end's up being a massive amount of transitive dependencies

[JCMais]

I'm wondering if it would be possible to just import the find method module more directly instead of from the root node-pre-gyp

I can do that

The usage of a dynamic require(..) call for locating the binding

This one I will have to test tho, I'm pretty sure having this static is going to cause issues during local development with node-pre-gyp build/rebuild commands. If this is the case it's probably possible to add a pre publish step to change this part with something else.

[JCMais]

Looks like this is in fact not necessary, the pre-release version available on node-libcurl@next is not using the find/dynamic require anymore. I will release it as 2.1.0 next week.

[JCMais]

I'm closing this as v2.1.0 (see the release page for full changelog) has been finally released. 🎉