-
Notifications
You must be signed in to change notification settings - Fork 927
feat: add condition for min node version #1782
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: add condition for min node version #1782
Conversation
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 weird that npx doesn't respect engines
entry in package.json. Until it does, I'm good with this check, just let's make it less of a perf impact
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.
Can you confirm this still works after extra imports? Ideally we'd have a test for that but It may be tricky to come up with a good one. Mocking process.version will not take module resolution into account and otherwise we'd need to run this through node@12
Yeah, I tested that with node@12 and it still works 😄 |
Summary:
Closes #1768
Test Plan:
node
e.g.v12.22.12
node packages/cli/build/bin.js start
inside repo and, you should get an errorYou need at least Node 14 to run CLI.