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

fix: Add support for Node 16 #2886

Merged
merged 1 commit into from
Jul 25, 2023
Merged

fix: Add support for Node 16 #2886

merged 1 commit into from
Jul 25, 2023

Conversation

benmccann
Copy link
Contributor

@benmccann benmccann commented Jul 22, 2023

Marked version: master

Markdown flavor: n/a

Description

Node 16 is still supported until September and there's no code in marked that doesn't work on Node 16, so it's unnecessarily aggressive to disallow it.

Expectation

Should be able to run marked on Node 16

Result

engines field gets in the way prior to this PR

Contributor

  • Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
  • no tests required for this PR.
  • If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

@vercel
Copy link

vercel bot commented Jul 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marked-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2023 8:18pm

Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

I’m surprised tests are passing. We probably need to bump the target output to truly drop node 16 come September https://nodejs.org/gl/blog/announcements/nodejs16-eol

@UziTech
Copy link
Member

UziTech commented Jul 24, 2023

Our docs state that we support lts and current node. The engines field should reflect what versions we support not what versions happen to work.

@benmccann
Copy link
Contributor Author

Well Node 16 is an LTS release: https://nodejs.dev/en/about/releases/. And in any case, we could easily change the language on https://github.com/markedjs/marked#compatibility to cover Node 16. It's pretty frustrating that this package works on Node 16 without issue and I'm being banned from running it there anyway. I don't quite understand the reasoning behind that or what the benefit is. And it seems like a lot of other people are frustrated too: #2767 (comment)

@UziTech
Copy link
Member

UziTech commented Jul 24, 2023

The reasoning is that none of us on the markedjs team want to debug issues on older versions of node. We have limited free time and would like to focus that time on versions of node we use. If you would like to join the team and help maintain older versions you are welcome to. We do appreciate the work you have done on marked 😁👍.

You can still use it. There are many well documented ways to get around the engines version. You can use --ignore-engines or create your own npm package that just depends on marked and uses babel to ensure a certain es output.

@benmccann
Copy link
Contributor Author

Sure, if there are any issues that arise on the oldest version of Node, but not more recent versions, I don't mind being responsible for those

@styfle
Copy link
Member

styfle commented Jul 24, 2023

Sounds easy enough - once 16.x added to CI then the issues will present themselves if a PR causes 16.x to regress 👍

@yoyo837
Copy link

yoyo837 commented Jul 25, 2023

Can this PR solve the problem mentioned in #2767 (comment)? Semantic release requires node v18.

@yoyo837
Copy link

yoyo837 commented Jul 25, 2023

@UziTech Can you help check this please?

@UziTech UziTech changed the title Add support for Node 16 fix: Add support for Node 16 Jul 25, 2023
@UziTech UziTech merged commit e465ce4 into markedjs:master Jul 25, 2023
github-actions bot pushed a commit that referenced this pull request Jul 25, 2023
## [5.1.2](v5.1.1...v5.1.2) (2023-07-25)

### Bug Fixes

* Add support for Node 16 ([#2886](#2886)) ([e465ce4](e465ce4))
@styfle styfle mentioned this pull request Jul 25, 2023
5 tasks
@benmccann benmccann deleted the node-16 branch July 25, 2023 14:14
@benmccann
Copy link
Contributor Author

thank you!!

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.

4 participants