-
-
Notifications
You must be signed in to change notification settings - Fork 203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(npm): add programmatic API for TypeScript #523
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #523 +/- ##
=======================================
Coverage 41.14% 41.14%
=======================================
Files 15 15
Lines 1038 1038
=======================================
Hits 427 427
Misses 611 611
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Tested it on both MacOS and Windows now. Also with a not 3 AM brain which let me to make the last 2 commits lol |
Hey this looks pretty good! I will test it locally soon. I can also create a RC release just to test this functionality and see if everything works with NPM. |
I tried running
Whereas this was working before like this:
@favna any idea what's going on? |
@orhun Fixed it. I overlooked the |
I do still get an error but I don't think that's related to the programmatic change.
In fact I switched back to main and I got this still, essentially the same error just less verbose (which relates to what Execa prints and I'd say in this case the higher verbosity is better)
|
I'm getting a different error:
However, I can confirm that the binary exists:
|
I applied this patch: diff --git a/npm/git-cliff/src/getExePath.ts b/npm/git-cliff/src/getExePath.ts
index f1bbbd6..341087f 100644
--- a/npm/git-cliff/src/getExePath.ts
+++ b/npm/git-cliff/src/getExePath.ts
@@ -28,7 +28,7 @@ export async function getExePath() {
);
} catch (e) {
throw new Error(
- `Couldn't find git-cliff binary inside node_modules for ${os}-${arch}`
+ `Couldn't find git-cliff binary inside node_modules for ${os}-${arch} (${e})`,
);
}
} The error is:
My IDE also complains about the
|
5254d1c
to
7b01b05
Compare
I was under the impression that I'll see if I can reproduce your issue. So far I've only tried with MacOS and Windows on Node 20. I'll try with Node 18.17.0 (your version as seen above) on both and leverage one of my VPSs for Linux x64 as well. I'll report back. |
I think the pattern is fairly obvious. Looks like there is a bug in Node Edit: I was curious which exactly version fixed it and it was
(as pulled from https://nodejs.org/download/release/) |
Thanks for debugging this - everything works as expected now after upgrading node! |
Description
Working on cliff-jumper it irked me that I had to call git-cliff through
npx
. Looking at the code I figured I could easily add a small programmatic API on top of (or technically rather below 😂) the CLI one currently provided so here we are.Other than the actual changes in the source code I also took the liberty to:
tsup
as bundler so separate CJS, ESM and CLI bundles can easily be generatedimport.meta.resolve
instead of the oldrequire.resolve
, yay!Important: This sets the minimum NodeJS requirement to Node 18 as it is the lowest version that supports
import.meta.resolve
without an experimental flag (docs v16, docs v18, docs v20). In my opinion this is acceptable because Node 16 has been End of Life since 2023-09-11 and people should as minimum update to Node 18.Motivation and Context
Having a programmatic API is always a nice-to-have for the more advanced users of the git-cliff npm package as it allows far easier scripting integration. The primary motivation here is my own library cliff-jumper which is one such "more advanced use".
How Has This Been Tested?
I have tested this by
yarn link
ing the local repository to my cliff-jumper repository where I then refactored my code to use the programmatic API. Where before I had to parse out the options provided to cliff-jumper to proper CLI options and call npx I can now call the programmatic API. I have ran this code and it works perfectly:Screenshots / Logs (if applicable)
N.A.
Types of Changes
Checklist:
(I hope so anyway, I had VSCode auto format and it should have picked up editorconfig)
I have formatted the code with rustfmt.N.A. I haven't touched rust codeclippyESLint.I have added tests to cover my changes.N.A.