fix(analyze): node-gyp-build not including zeromq prebuilds by eulersson · Pull Request #392 · vercel/nft

eulersson · 2024-02-13T16:23:47Z

Fixes Add support for zeromq #391

Three things I want to point out:

In zeromq the way node-gyp-build is called is by passing path.join(__dirname, "..") instead of __dirname to the require("node-gyp-build")(...) call.
In zeromq they use @aminya/node-gyp-build instead of node-gyp-build, particular-li version 4.5.0-aminya.5.
The zeromq version that's recommended for new project is the 6 which is beta according to @aminya https://github.com/zeromq/zeromq.js.

After those changes it resolves OK.

❯ node ./node_modules/@vercel/nft/out/cli.js print ./node_modules/zeromq/lib/index.js
FILELIST:
node_modules/@aminya/node-gyp-build/index.js
node_modules/@aminya/node-gyp-build/package.json
node_modules/zeromq/lib/draft.js
node_modules/zeromq/lib/index.js
node_modules/zeromq/lib/native.js
node_modules/zeromq/lib/util.js
node_modules/zeromq/package.json
node_modules/zeromq/prebuilds/darwin-x64/node.napi.glibc.node <---- It worked!

…evel where de require() is run at. Fixes zeromq.js. Closes vercel#391.

test/unit.test.js

test/unit/zeromq-node-gyp/input.js

eulersson · 2024-02-13T17:16:40Z

I will fix the test havoc ⛈️

test/unit/zeromq-node-gyp/output.js

package.json

src/analyze.ts

aminya · 2024-02-13T19:36:26Z

@eulersson Thanks for making a fix. Let me know if there is anything to do on the Zeromq side.

…tead log them as warnings to prevent user confusion.

…ttps://github.com/eulersson/nft into fix-node-gyp-build-not-including-zeromq-prebuilds

eulersson · 2024-02-13T20:15:33Z

@eulersson Thanks for making a fix. Let me know if there is anything to do on the Zeromq side.

No problem. For now I do not think it will be needed.

styfle

Great work, thanks! 🎉

github-actions · 2024-02-13T22:42:43Z

🎉 This PR is included in version 0.26.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Fix node-gyp-build not including prebuilds that are not at the same l…

c094d28

…evel where de require() is run at. Fixes zeromq.js. Closes vercel#391.

eulersson requested review from ijjk and styfle as code owners February 13, 2024 16:23

eulersson changed the title ~~node-gyp-build not including zeromq prebuilds~~ fix(resolution) node-gyp-build not including zeromq prebuilds Feb 13, 2024

eulersson changed the title ~~fix(resolution) node-gyp-build not including zeromq prebuilds~~ fix(resolution): node-gyp-build not including zeromq prebuilds Feb 13, 2024

Added unit tests.

d8f2188