refactor: use bindings to load .node file

[codebytere]

This PR alters .node file loading to use bindings. I'd like to make this change because it enables Webpack to understand how to handle this module, a need which would apply to, for example, Electron apps which both need to use native node modules and use Webpack.

This should not disrupt any existing functionality, and bindings itself only has one dependency, which makes this change have minimal impact as far as added dependencies are concerned.

Thank you!

[kewde]

Thank you for your contribution @codebytere

I'm closing and re-opening the PR to trigger the CI again because I noticed some false positives.
The failures in the CI seem unrelated to any of these changes.

[codebytere]

@kewde i think they're flakes too - please let me know if there's anything else i need to take care of for this PR otherwise :)

[kewde]

I've had the time to inspect this a little bit deeper.

The behavior of bindings is to check all the possible locations that a native addon would be built at, and returns the first one that loads successfully.

The behavior of find() in node-pre-gyp builds up the path using information from package.json, the node runtime, etc. In short, find() will guarantee that it tries to load the correct version.

If everything goes well and only the right bindings are downloaded/built then this shouldn't cause any problems. But I've seen many situations where people (myself included) have updated their Node environment (e.g upgraded from Node 12 to Node 13) but the bindings (.node files) of the older version are not cleaned up and still available in the directory. They are not cleaned up properly. I suspect this PR may cause the loading of older bindings.

[codebytere]

@kewde do you see a path forward here in that event?

[kewde]

@codebytere I just hope the bindings library fails to load .node bindings that were compiled for an older ABI version. I will test this out, if it fails to load the older bindings then the behavior remains unchanged.

I don't see it as a blocker for this pull request though.

[kewde]

@codebytere
Does using #1268 also solve the webpack issue?
I'm more leaning to accepting that one simply because:

1. no additional dependency
2. same behavior as before

Please let me know if I'm missing something.
Thank you.

[codebytere]

This PR alters .node file loading to use bindings. I'd like to make this change because it enables Webpack to understand how to handle this module, a need which would apply to, for example, Electron apps which both need to use native node modules and use Webpack.

This should not disrupt any existing functionality, and bindings itself only has one dependency, which makes this change have minimal impact as far as added dependencies are concerned.

Thank you!

[kewde]

Thank you for your contribution @codebytere

I'm closing and re-opening the PR to trigger the CI again because I noticed some false positives.
The failures in the CI seem unrelated to any of these changes.

[codebytere]

@kewde i think they're flakes too - please let me know if there's anything else i need to take care of for this PR otherwise :)

[kewde]

I've had the time to inspect this a little bit deeper.

The behavior of bindings is to check all the possible locations that a native addon would be built at, and returns the first one that loads successfully.

The behavior of find() in node-pre-gyp builds up the path using information from package.json, the node runtime, etc. In short, find() will guarantee that it tries to load the correct version.

If everything goes well and only the right bindings are downloaded/built then this shouldn't cause any problems. But I've seen many situations where people (myself included) have updated their Node environment (e.g upgraded from Node 12 to Node 13) but the bindings (.node files) of the older version are not cleaned up and still available in the directory. They are not cleaned up properly. I suspect this PR may cause the loading of older bindings.

[codebytere]

@kewde do you see a path forward here in that event?

[kewde]

@codebytere I just hope the bindings library fails to load .node bindings that were compiled for an older ABI version. I will test this out, if it fails to load the older bindings then the behavior remains unchanged.

I don't see it as a blocker for this pull request though.

[kewde]

@codebytere
Does using #1268 also solve the webpack issue?
I'm more leaning to accepting that one simply because:

1. no additional dependency
2. same behavior as before

Please let me know if I'm missing something.
Thank you.

[codebytere]

This PR alters .node file loading to use bindings. I'd like to make this change because it enables Webpack to understand how to handle this module, a need which would apply to, for example, Electron apps which both need to use native node modules and use Webpack.

This should not disrupt any existing functionality, and bindings itself only has one dependency, which makes this change have minimal impact as far as added dependencies are concerned.

Thank you!

[kewde]

Thank you for your contribution @codebytere

I'm closing and re-opening the PR to trigger the CI again because I noticed some false positives.
The failures in the CI seem unrelated to any of these changes.

[codebytere]

@kewde i think they're flakes too - please let me know if there's anything else i need to take care of for this PR otherwise :)

[kewde]

I've had the time to inspect this a little bit deeper.

The behavior of bindings is to check all the possible locations that a native addon would be built at, and returns the first one that loads successfully.

The behavior of find() in node-pre-gyp builds up the path using information from package.json, the node runtime, etc. In short, find() will guarantee that it tries to load the correct version.

If everything goes well and only the right bindings are downloaded/built then this shouldn't cause any problems. But I've seen many situations where people (myself included) have updated their Node environment (e.g upgraded from Node 12 to Node 13) but the bindings (.node files) of the older version are not cleaned up and still available in the directory. They are not cleaned up properly. I suspect this PR may cause the loading of older bindings.

[codebytere]

@kewde do you see a path forward here in that event?

[kewde]

@codebytere I just hope the bindings library fails to load .node bindings that were compiled for an older ABI version. I will test this out, if it fails to load the older bindings then the behavior remains unchanged.

I don't see it as a blocker for this pull request though.

[kewde]

@codebytere
Does using #1268 also solve the webpack issue?
I'm more leaning to accepting that one simply because:

1. no additional dependency
2. same behavior as before

Please let me know if I'm missing something.
Thank you.

[codebytere]

This PR alters .node file loading to use bindings. I'd like to make this change because it enables Webpack to understand how to handle this module, a need which would apply to, for example, Electron apps which both need to use native node modules and use Webpack.

This should not disrupt any existing functionality, and bindings itself only has one dependency, which makes this change have minimal impact as far as added dependencies are concerned.

Thank you!

[kewde]

Thank you for your contribution @codebytere

I'm closing and re-opening the PR to trigger the CI again because I noticed some false positives.
The failures in the CI seem unrelated to any of these changes.

[codebytere]

@kewde i think they're flakes too - please let me know if there's anything else i need to take care of for this PR otherwise :)

[kewde]

I've had the time to inspect this a little bit deeper.

The behavior of bindings is to check all the possible locations that a native addon would be built at, and returns the first one that loads successfully.

The behavior of find() in node-pre-gyp builds up the path using information from package.json, the node runtime, etc. In short, find() will guarantee that it tries to load the correct version.

If everything goes well and only the right bindings are downloaded/built then this shouldn't cause any problems. But I've seen many situations where people (myself included) have updated their Node environment (e.g upgraded from Node 12 to Node 13) but the bindings (.node files) of the older version are not cleaned up and still available in the directory. They are not cleaned up properly. I suspect this PR may cause the loading of older bindings.

[codebytere]

@kewde do you see a path forward here in that event?

[kewde]

@codebytere I just hope the bindings library fails to load .node bindings that were compiled for an older ABI version. I will test this out, if it fails to load the older bindings then the behavior remains unchanged.

I don't see it as a blocker for this pull request though.

[kewde]

@codebytere
Does using #1268 also solve the webpack issue?
I'm more leaning to accepting that one simply because:

1. no additional dependency
2. same behavior as before

Please let me know if I'm missing something.
Thank you.

[codebytere]

This PR alters .node file loading to use bindings. I'd like to make this change because it enables Webpack to understand how to handle this module, a need which would apply to, for example, Electron apps which both need to use native node modules and use Webpack.

This should not disrupt any existing functionality, and bindings itself only has one dependency, which makes this change have minimal impact as far as added dependencies are concerned.

Thank you!

[kewde]

Thank you for your contribution @codebytere

I'm closing and re-opening the PR to trigger the CI again because I noticed some false positives.
The failures in the CI seem unrelated to any of these changes.

[codebytere]

@kewde i think they're flakes too - please let me know if there's anything else i need to take care of for this PR otherwise :)

[kewde]

I've had the time to inspect this a little bit deeper.

The behavior of bindings is to check all the possible locations that a native addon would be built at, and returns the first one that loads successfully.

The behavior of find() in node-pre-gyp builds up the path using information from package.json, the node runtime, etc. In short, find() will guarantee that it tries to load the correct version.

If everything goes well and only the right bindings are downloaded/built then this shouldn't cause any problems. But I've seen many situations where people (myself included) have updated their Node environment (e.g upgraded from Node 12 to Node 13) but the bindings (.node files) of the older version are not cleaned up and still available in the directory. They are not cleaned up properly. I suspect this PR may cause the loading of older bindings.

[codebytere]

@kewde do you see a path forward here in that event?

[kewde]

@codebytere I just hope the bindings library fails to load .node bindings that were compiled for an older ABI version. I will test this out, if it fails to load the older bindings then the behavior remains unchanged.

I don't see it as a blocker for this pull request though.

[kewde]

@codebytere
Does using #1268 also solve the webpack issue?
I'm more leaning to accepting that one simply because:

1. no additional dependency
2. same behavior as before

Please let me know if I'm missing something.
Thank you.

[codebytere]

This PR alters .node file loading to use bindings. I'd like to make this change because it enables Webpack to understand how to handle this module, a need which would apply to, for example, Electron apps which both need to use native node modules and use Webpack.

This should not disrupt any existing functionality, and bindings itself only has one dependency, which makes this change have minimal impact as far as added dependencies are concerned.

Thank you!

[kewde]

Thank you for your contribution @codebytere

I'm closing and re-opening the PR to trigger the CI again because I noticed some false positives.
The failures in the CI seem unrelated to any of these changes.

[codebytere]

@kewde i think they're flakes too - please let me know if there's anything else i need to take care of for this PR otherwise :)

[kewde]

I've had the time to inspect this a little bit deeper.

The behavior of bindings is to check all the possible locations that a native addon would be built at, and returns the first one that loads successfully.

The behavior of find() in node-pre-gyp builds up the path using information from package.json, the node runtime, etc. In short, find() will guarantee that it tries to load the correct version.

If everything goes well and only the right bindings are downloaded/built then this shouldn't cause any problems. But I've seen many situations where people (myself included) have updated their Node environment (e.g upgraded from Node 12 to Node 13) but the bindings (.node files) of the older version are not cleaned up and still available in the directory. They are not cleaned up properly. I suspect this PR may cause the loading of older bindings.

[codebytere]

@kewde do you see a path forward here in that event?

[kewde]

@codebytere I just hope the bindings library fails to load .node bindings that were compiled for an older ABI version. I will test this out, if it fails to load the older bindings then the behavior remains unchanged.

I don't see it as a blocker for this pull request though.

[kewde]

@codebytere
Does using #1268 also solve the webpack issue?
I'm more leaning to accepting that one simply because:

1. no additional dependency
2. same behavior as before

Please let me know if I'm missing something.
Thank you.

[codebytere]

This PR alters .node file loading to use bindings. I'd like to make this change because it enables Webpack to understand how to handle this module, a need which would apply to, for example, Electron apps which both need to use native node modules and use Webpack.

This should not disrupt any existing functionality, and bindings itself only has one dependency, which makes this change have minimal impact as far as added dependencies are concerned.

Thank you!

[kewde]

Thank you for your contribution @codebytere

I'm closing and re-opening the PR to trigger the CI again because I noticed some false positives.
The failures in the CI seem unrelated to any of these changes.

[codebytere]

@kewde i think they're flakes too - please let me know if there's anything else i need to take care of for this PR otherwise :)

[kewde]

I've had the time to inspect this a little bit deeper.

The behavior of bindings is to check all the possible locations that a native addon would be built at, and returns the first one that loads successfully.

The behavior of find() in node-pre-gyp builds up the path using information from package.json, the node runtime, etc. In short, find() will guarantee that it tries to load the correct version.

If everything goes well and only the right bindings are downloaded/built then this shouldn't cause any problems. But I've seen many situations where people (myself included) have updated their Node environment (e.g upgraded from Node 12 to Node 13) but the bindings (.node files) of the older version are not cleaned up and still available in the directory. They are not cleaned up properly. I suspect this PR may cause the loading of older bindings.

[codebytere]

@kewde do you see a path forward here in that event?

[kewde]

@codebytere I just hope the bindings library fails to load .node bindings that were compiled for an older ABI version. I will test this out, if it fails to load the older bindings then the behavior remains unchanged.

I don't see it as a blocker for this pull request though.

[kewde]

@codebytere
Does using #1268 also solve the webpack issue?
I'm more leaning to accepting that one simply because:

1. no additional dependency
2. same behavior as before

Please let me know if I'm missing something.
Thank you.

[codebytere]

This PR alters .node file loading to use bindings. I'd like to make this change because it enables Webpack to understand how to handle this module, a need which would apply to, for example, Electron apps which both need to use native node modules and use Webpack.

This should not disrupt any existing functionality, and bindings itself only has one dependency, which makes this change have minimal impact as far as added dependencies are concerned.

Thank you!

[kewde]

Thank you for your contribution @codebytere

I'm closing and re-opening the PR to trigger the CI again because I noticed some false positives.
The failures in the CI seem unrelated to any of these changes.

[codebytere]

@kewde i think they're flakes too - please let me know if there's anything else i need to take care of for this PR otherwise :)

[kewde]

I've had the time to inspect this a little bit deeper.

The behavior of bindings is to check all the possible locations that a native addon would be built at, and returns the first one that loads successfully.

The behavior of find() in node-pre-gyp builds up the path using information from package.json, the node runtime, etc. In short, find() will guarantee that it tries to load the correct version.

If everything goes well and only the right bindings are downloaded/built then this shouldn't cause any problems. But I've seen many situations where people (myself included) have updated their Node environment (e.g upgraded from Node 12 to Node 13) but the bindings (.node files) of the older version are not cleaned up and still available in the directory. They are not cleaned up properly. I suspect this PR may cause the loading of older bindings.

[codebytere]

@kewde do you see a path forward here in that event?

[kewde]

@codebytere I just hope the bindings library fails to load .node bindings that were compiled for an older ABI version. I will test this out, if it fails to load the older bindings then the behavior remains unchanged.

I don't see it as a blocker for this pull request though.

[kewde]

@codebytere
Does using #1268 also solve the webpack issue?
I'm more leaning to accepting that one simply because:

1. no additional dependency
2. same behavior as before

Please let me know if I'm missing something.
Thank you.