Skip to content

Conversation

@kinggoesgaming
Copy link
Contributor

@kinggoesgaming kinggoesgaming commented Jul 21, 2024

  • refactor: replace chalk with styleText
  • build(deps): remove chalk
  • style(imports): fix import order

Test Plan

No tests or docs need changing

Screenshots

No change that requires diff of screenshots

closes: #792

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super!

@jacobparis
Copy link
Contributor

styleText doesn't land until Node 22 (node 21.7 technically) which won't be LTS until October, so I don't think we can merge this until then based on this decision doc

https://github.com/epicweb-dev/epic-stack/blob/main/docs/decisions/021-node-version.md

@kinggoesgaming
Copy link
Contributor Author

styleText doesn't land until Node 22 (node 21.7 technically) which won't be LTS until October, so I don't think we can merge this until then based on this decision doc

https://github.com/epicweb-dev/epic-stack/blob/main/docs/decisions/021-node-version.md

I am so used to using it that I didn't realize it is still in Active development stage.

I will try to keep this PR up-to-date until it is stablized.

@kinggoesgaming
Copy link
Contributor Author

Another point: this does not actually remove chalk from the dependency tree, as other dependencies do depend on chalk, but now it is a transitive dev dependency.

Signed-off-by: Hunar Roop Kahlon <[email protected]>
Signed-off-by: Hunar Roop Kahlon <[email protected]>
@kinggoesgaming kinggoesgaming force-pushed the 792-use-styletext-from-node-util branch from 353f1d2 to 181f7e2 Compare August 3, 2024 20:19
@alcpereira
Copy link
Contributor

alcpereira commented Aug 4, 2024

styleText doesn't land until Node 22 (node 21.7 technically) which won't be LTS until October, so I don't think we can merge this until then based on this decision doc

https://github.com/epicweb-dev/epic-stack/blob/main/docs/decisions/021-node-version.md

What do you think about updating the requirement to Node 20.12.0?

I'm used to work with .nvmrc or .node-version files so that Node version manager like fnm can select the right version for the project.
Is it worth opening a PR to add one of those (with corresponding documentation)? So it might also unblock this PR.
Edit: This is already suggested in #799

@kentcdodds
Copy link
Member

Why don't node managers reference the engines config in the package.json? I'd really rather avoid adding an extra file to the root just for the node version.

@kentcdodds
Copy link
Member

I'm going to close this for now. We can revisit when Node 22 is our base.

@kentcdodds kentcdodds closed this Aug 27, 2024
@kentcdodds kentcdodds reopened this Feb 3, 2025
@kentcdodds
Copy link
Member

Now we're running on node version 22. So we can look into doing this. There have been a number of changes to these different files. So if you would like to update your pull request with what is latest on main and get everything ready to go then that would be really appreciated. And then we can get this merged in. Thanks.

@kinggoesgaming
Copy link
Contributor Author

I can look into this in a few days...

@kinggoesgaming kinggoesgaming mentioned this pull request Feb 13, 2025
@kinggoesgaming
Copy link
Contributor Author

superceded by #928

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 this pull request may close these issues.

Use styleText from node:util

4 participants