Skip to content

fix: Add support for Node 16#2886

Merged
UziTech merged 1 commit intomarkedjs:masterfrom
benmccann:node-16
Jul 25, 2023
Merged

fix: Add support for Node 16#2886
UziTech merged 1 commit intomarkedjs:masterfrom
benmccann:node-16

Conversation

@benmccann
Copy link
Copy Markdown
Contributor

@benmccann benmccann commented Jul 22, 2023 •
edited
Loading

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
Copy Markdown

vercel bot commented Jul 22, 2023 •
edited
Loading

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

@benmccann benmccann mentioned this pull request Jul 22, 2023
Merged
5 tasks
styfle
styfle approved these changes Jul 24, 2023
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Member

UziTech commented Jul 24, 2023 •
edited
Loading

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
Copy Markdown
Contributor Author

benmccann commented Jul 24, 2023

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
Copy Markdown
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
Copy Markdown
Contributor Author

benmccann commented Jul 24, 2023

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
Copy Markdown
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
Copy Markdown

yoyo837 commented Jul 25, 2023

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

@yoyo837
Copy link
Copy Markdown

yoyo837 commented Jul 25, 2023 •
edited
Loading

@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
@semantic-release-bot
chore(release): 5.1.2 [skip ci]
929d235 
## [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
Merged
5 tasks
@benmccann benmccann deleted the node-16 branch July 25, 2023 14:14
@benmccann
Copy link
Copy Markdown
Contributor Author

benmccann commented Jul 25, 2023

thank you!!

@X-oss-byte X-oss-byte mentioned this pull request May 24, 2024
Open
@X-oss-byte X-oss-byte mentioned this pull request Sep 7, 2024
Open
Logiclayer1111 pushed a commit to Logiclayer1111/marked that referenced this pull request Apr 20, 2026
@semantic-release-bot
chore(release): 5.1.2 [skip ci]
e860d44 
## [5.1.2](markedjs/marked@v5.1.1...v5.1.2) (2023-07-25)

### Bug Fixes

* Add support for Node 16 ([#2886](markedjs/marked#2886)) ([1911642](markedjs/marked@1911642))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UziTech UziTech UziTech approved these changes

@styfle styfle styfle approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@benmccann @UziTech @styfle @yoyo837