cronvel / terminal-kit

Terminal utilities for node.js
MIT License
3.11k stars 201 forks source link

terminal-kit 2.2.0 broke installs for Node < 14 #192

Closed DanielRuf closed 2 years ago

DanielRuf commented 2 years ago

It seems string-kit 0.14.x requires Node 14 or newer. https://github.com/cronvel/terminal-kit/blame/v2.2.3/package.json#L19

So this breaks projects which use for example foundation-cli. See https://github.com/foundation/foundation-sites/discussions/12340#discussioncomment-1739023

The change / upgrade was done in terminal-kit 2.2.0:

https://github.com/cronvel/terminal-kit/blob/v2.1.8/package.json#L19 https://github.com/cronvel/terminal-kit/blob/v2.2.0/package.json#L19

This should have been a 3.0 release according to SemVer (semantic versioning). Best is to revert and do the change in terminal-kit 3.0.0 or newer.

The relevant changes that were done in string-kit 0.14.0:

https://github.com/cronvel/string-kit/blob/v0.14.0/lib/format.js#L327 https://github.com/cronvel/string-kit/commit/462665ee5b647e48894971ed332cc825e395c3db

cronvel commented 2 years ago

@DanielRuf Nope, Terminal-Kit requires Node.js 14.15.0 (Fermium LTS) since version 2.0.0, and the engine upgrade was the only reason for that SEMVER MAJOR. CHANGELOG Even on your own link, if you scroll up you will see:

  "engines": {
    "node": ">=14.15.0"
  },

You shouldn't use Terminal-Kit if you are not on Node >= 14.15 and it is pure luck that it didn't crash before.

DanielRuf commented 2 years ago

@cronvel the trick with engines does not work. You can try that with npm i -g foundation-cli && foundation new on Node 12 for example.

DanielRuf commented 2 years ago

Anyways, I have found the commit which bumped the version in the CLI at https://github.com/foundation/foundation-cli/commit/d619c4fc4bc1dd81f9bbabdabd0f539c6a127024 and let @joeworkman as maintainer know about it.

Afaik npm doesn't prevent install of packages with an engines setting and only warns. See also https://docs.npmjs.com/cli/v8/configuring-npm/package-json#engines

DanielRuf commented 2 years ago

Nope, Terminal-Kit requires Node.js 14.15.0 (Fermium LTS) since version 2.0.0

I probably oversaw that. Thank you very much.

it is pure luck that it didn't crash before.

You are right, the upgrade of terminal-kit should not have happened in foundation-cli in the first place.

cronvel commented 2 years ago

@cronvel the trick with engines does not work. You can try that with npm i -g foundation-cli && foundation new on Node 12 for example. Afaik npm doesn't prevent install of packages with an engines setting and only warns.

I know and it's a shame, but you should blame npm for that. And also foundation-cli for not checking the changelog when doing a SEMVER MAJOR dependency upgrade.

Everything is OK on my end, and there is nothing I can do to improve the situation, that's why I closed the issue.

Good luck! ;)

DanielRuf commented 2 years ago

And also foundation-cli for not checking the changelog when doing a SEMVER MAJOR dependency upgrade. Everything is OK on my end, and there is nothing I can do to improve the situation, that's why I closed the issue.

You are right. Thanks for your time and feedback.