-
Notifications
You must be signed in to change notification settings - Fork 43
feat!: cliui is now ESM only #165
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
Conversation
Deno related code updated to support wrapping and test now passing. A high level question is with the Deno support for node packages so good, is there any benefit to continuing to publish a deno build separately? |
Amazing @shadowspawn thank you for doing this 👏 I was dreading having to do so. |
I agree, we might want to stop doing this. Perhaps in the future after we finish this migration to ESM though? |
}, | ||
"files": [ | ||
"build", | ||
"index.mjs", | ||
"!*.d.ts" | ||
], | ||
"engines": { | ||
"node": ">=12" | ||
"node": ">=20" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm bumping into some issues in yargs with Node@<20.19.0
, should we consider making this our base engine version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is 20.19.0 around something in particular? require(esm) ?
In which case, we might want a minimum for node 22 as well.
I am not against the idea of minimum versions within a major when there is a reason. (I need to remind myself that the early minor versions were before LTS status, so clearly not all equal! 😆 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't notice this had been merged when I made this command. No reply necessary!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment and potential change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's reasonable that cliui could have a looser version range supported than yargs.
Highlights:
Wrapping issues: #89 #138
Wrapping PRs: #139 #143
See also yargs issue for ESM only: yargs/yargs#2451
Proposing require Node.js 20 since 18 is dropping out of LTS on 2025-04-30. Hopefully 20 will have require(esm) by default in v20.19.0: nodejs/node#57349
To do: