node-cloudflared icon indicating copy to clipboard operation
node-cloudflared copied to clipboard

Install bin in npm postinstall

Open SpainTrain opened this issue 1 year ago • 1 comments

First - really appreciate this library, it has saved a ton of time in my project!

It appears that the cloudflared binary installation - https://github.com/JacobLinCool/node-cloudflared/blob/b45b5fc06d0066c3fe3aded4cb3524374adf09a0/src/install.ts - to be performed in the postinstall hook - https://github.com/JacobLinCool/node-cloudflared/blob/b45b5fc06d0066c3fe3aded4cb3524374adf09a0/scripts/postinstall.mjs - rather than as a separate step out-of-band.

I am curious:

  • Is there a use case this separation is supporting (I am guessing maybe version selection based on https://github.com/JacobLinCool/node-cloudflared/issues/2
  • Would you be open to a PR to use the postinstall hook approach, as long as it also supported any additional use cases the current approach supports?

Cheers!

SpainTrain avatar Jan 26 '24 23:01 SpainTrain

Hi @SpainTrain.

Is there a use case this separation is supporting (I am guessing maybe version selection based on Install specified version of cloudflared #2

No, the separation is a workaround for the dependency problem during the development setup. When running pnpm i in the development environment, the lib directory has not been transpiled, so directly importing it will cause the installation to fail. That's why scripts/postinstall.mjs needs to run (in the development environment) before installing the cloudflared binary.

Would you be open to a PR to use the postinstall hook approach, as long as it also supported any additional use cases the current approach supports?

Yes, I think it's possible to simplify the postinstall script from node scripts/postinstall.mjs && node lib/cloudflared.js -v to something like node scripts/postinstall.mjs by dynamically importing lib/install.js after the required build process (if in the development environment).

JacobLinCool avatar Jan 27 '24 13:01 JacobLinCool