Skip to content
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

Addition of @alcalzone/ansi-tokenize is a breaking change as it only supports Node 16+ #617

Closed
newhouse opened this issue Aug 4, 2023 · 5 comments · Fixed by #618
Closed

Comments

@newhouse
Copy link
Contributor

newhouse commented Aug 4, 2023

Hello and thank you for this library! I recently came across a problem, however.

It looks like a few weeks ago @alcalzone/ansi-tokenize was added to this package.

The problem is that while this package supports Node 14, unfortunately @alcalzone/ansi-tokenize does not.

For fresh installations of things that have ink as a dependency and there is no "old" lock file around, this might cause an error during yarn/npm install on Node 14.

Here is an example of it happening for my package that I maintain: https://github.com/anvilco/spectaql/actions/runs/5766329255/job/15634072138?pr=860

And here is a screenshot of it happening when I add ink as a dependency to a clean Node 14 project that did not previously have ink in the lock file:
image

There are a bunch of directions you can go, but a few that come to mind are:

  • Change the engine requirement on @alcalzone/ansi-tokenize to include Node 14...if it will support it, then once it's published update the min version of it here and publish.
  • Remove that as a dependency and do whatever you were doing before without it
  • Revert the addition of it, publish a new minor version where the engines are not violated, then cut a new major version of ink that now requires a higher node version

I hope that option 1 or 2 are chosen...but either way, something should be changed here because your engines specification is no longer correct.

@vadimdemedes @AlCalzone

@vadimdemedes
Copy link
Owner

@newhouse Thanks for reporting this!

@AlCalzone Would you be open to reverting AlCalzone/ansi-tokenize@621b28e? Was there a particular reason why Node.js v14.x support was dropped?

I checked @alcalzone/ansi-tokenize dependencies and both ansi-styles and is-fullwidth-code-point support Node.js v14.x, so fortunately it's only a matter of allowing Node.js v14.x in @alcalzone/ansi-tokenize.

@AlCalzone
Copy link
Contributor

I'll check it out. Shouldn't be a big deal supporting node 14.

@AlCalzone
Copy link
Contributor

v0.1.2 should be out in a minute or two and requires node 14.13.1, which should be compatible with ink

@newhouse
Copy link
Contributor Author

newhouse commented Aug 7, 2023

Thank you @AlCalzone I can confirm that my projects and example builds are working again on Node 14 when 0.1.2 is the resolved version for @alcalzone/ansi-tokenize.

For most situations, their setups will probably just work already...but for things to be predictably correct, it's probably a good idea to bump the dependency and then publish a new patch version with that new version.

I've created a PR for this repo that bumps the min version for @alcalzone/ansi-tokenize to ^0.1.2: #618

I think all that's left is for that to be merged, then a patch release of this by @vadimdemedes

@vadimdemedes
Copy link
Owner

Thanks guys! New release is out → https://github.com/vadimdemedes/ink/releases/tag/v4.3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants