Breaking Change in 2.1.1

[Fleker]

Marked version: 2.1.1

Describe the bug
Version 2.1.1 drops support both for Node 8 and Node 10, mandating Node 12 and higher. This is a sudden change that violates semantic versioning by cutting off support for previously working versions. Doing this change to the engines field in a patch makes it very hard to catch ahead of time, but does result in breaking our CI pipeline via Typedoc. Our library and Typedoc both have a minimum of Node 10.

+ yarn
# ...
error marked@2.1.1: The engine "node" is incompatible with this module. Expected version ">= 12". Got "10.18.0"
error Found incompatible module.

To Reproduce
Steps to reproduce the behavior:

• Start with an environment using Node 10 (nvm use 10.18.0)
• Create a project with a dependency of typedoc@^0.20.0 or marked@^2.00
• Run yarn

Expected behavior
A clear and concise description of what you expected to happen.

I expect that a patch release will maintain existing support for Node platforms and that any deprecation is both clearly documented and breaking changes are placed in a major release.

Can you please revert this change to engines and release a 2.1.2 as soon as possible? I understand the value in dropping older versions, but this change was sudden and are having effects on dependents.

[UziTech]

Node v4+ is supported with const marked = require('marked/lib/marked.js')

This is why engines field is misleading and shouldn't be trusted. We explain our semver numbering in our docs. Only node latest (currently v16) and latest lts (currently v14) are guaranteed to be supported at any time.

[UziTech]

We definitely don't support, nor want to encourage, insecure versions of node.

[Fleker]

I understand that Node 10 is now EOL, but making this change in a patch release has broken our build via dependencies in a way that we could not catch. Because this is being installed in our environment via typedoc and not directly, there is a certain reliance on this package that has downstream implications in the ecosystem.

According to your documentation, a patch release should be backwards compatible.

Patch: Changes that move Marked closer to spec compliance or change a public API that does not break backwards compatibility.

In the case of 2.1.1, there is definitively a breaking of compatibility. Using my environment I used to be able to install ^2.0.0. Now I cannot. However, because we aren't installing marked directly, our ability to mitigate this breaking change is even more restricted. (Thankfully in our case we were able to patch this in Typedoc as an immediate fix.)

Yes, our project should be updating to a supported version of Node, but that is not a process that we can do overnight as it would require us to make a major release in order to broadcast to our dependents that they cannot just accept the latest minor/patch version because trying to do so would break their builds.

To suggest that engines is misleading and shouldn't be trusted is not simply a package-level decision, as yarn interprets engines in a very particular way based on the official spec. This error is intentional as it catches potential breaking issues within the code itself such as optional chaining that would also break the build. yarn install --ignore-engines could be a quick fix, but could still cause syntax changes to cause a problem.

I would ask that you please consider reverting this change to engines in the 2.1.x version, as any dependent and subdependent simply pulling in ^2.0.0 may get a breaking change. You can end support for 2.x versions and release a 3.x that would be able to drop support for EOL versions of Node while also broadcasting to dependents that they may not be able to simply update to the newest version in their existing environment.

[UziTech]

Marked v2.1.1 does work in node v10 therefore there are no breaking changes. The engines field has no absolute spec as far as I know (if it does please share a link). It does different things depending on what installer is used.

The burden of creating a new major version is no different in marked than in other packages.

Yarn has ways of pinning dependencies.

[Fleker]

Based on my reading of the engines documentation, it specifies the version of node that a dependency can work in a machine-readable format if defined. It doesn't have to be defined, or a "*" can indicate any version. This is why yarn would check the field's value before running the installation. Given the popularity of yarn, their interpretation of the field is significant for the ecosystem.

There are ways to mitigate this error, but modifying the install behavior in a way that causes my CI build to fail seems to be a breaking change from my perspective. It is also surprising such a change was introduced in a patch release.

If you do not have a significant opinion in the value of the engines field, it may be better to remove it to avoid this error continuing or consider reverting the change for the next patch release.

[UziTech]

If you do not have a significant opinion in the value of the engines field, it may be better to remove it to avoid this error

I have advocated for removing it but some in the community believe it is necessary. @styfle

I personally don't use yarn very often so it doesn't affect me one way or the other.

[styfle]

I advocate for dropping support for a Node.js version in semver major to avoid all these issues.

The reason why engines is useful is to indicate which versions are supported in a formal way.

[joshbruce]

I advocate for dropping support for a Node.js version in semver major to avoid all these issues.

I second this - @styfle beat me to this. It may also be beneficial for transparency to define our EOL strategy.

How long will versions of Node be supported? Up to EOL? EOL + 6 months? And so on.

[UziTech]

How long will versions of Node be supported? Up to EOL? EOL + 6 months? And so on.

This is the type of thing we wouldn't need to worry about if we remove the engines field. Marked is a community package and should support what ever version someone is willing to support.

I'm only willing to support latest lts and latest node versions, and a few of my projects were still using node v12 (I have updated them since) which is why I submitted a PR to fix v12 from v2.1.0.

If there is a person willing to review PRs and make sure they work with node v8 they should be able to.

[styfle]

This is the type of thing we wouldn't need to worry about if we remove the engines field

I don't believe that is true which is why issue #2106 was created. The issue was that Node 12 wasn't tested.

If there is a person willing to review PRs and make sure they work with node v8 they should be able to.

I don't think we need to designate a person, this is the job of CI. As long as the lowest common denominator is running the tests automatically (Node.js 12), it shouldn't be a problem.

The only step remaining is to ensure that semver major is bumped when removing Node 12 from CI.

[UziTech]

The only step remaining is to ensure that semver major is bumped when removing Node 12 from CI

Which is going to be in a few years because no one is going to remember to remove it when node v12 EOL comes around.

The biggest problem with hard coding node versions in CI is that no one remembers to update them. When node v16 becomes lts than we will be testing node v12, v16 and v17 not v14 any more. And we probably aren't going to test v14 until something breaks it.

If this were a project backed by a company like react or babel and had people working on it every day it might be a different story. For those projects I think engines field and testing specific node versions is useful, but marked is not like those projects and there can be months where no one is working on it.

I don't believe that is true which is why issue #2106 was created. The issue was that Node 12 wasn't tested.

exactly, so the engines field was useless in this situation.

[calculuschild]

Or alternatively... We just utilize Babel provide a version of Marked that by default is compatible with older Node versions without having to manage those node versions ourselves.

[UziTech]

Or we say marked works with lts and latest node and tell dependents they need to transpile marked if they need it to work with a specific version of node.

[styfle]

The biggest problem with hard coding node versions in CI is that no one remembers to update them

But there is no harm with not updating CI because then we continue supporting old Node.js versions and thats okay.

The problem is when we're not testing a specific Node.js version and accidentally remove support for it.

[UziTech]

The problem is when we're not testing a specific Node.js version and accidentally remove support for it.

Right, like when we accidentally remove support for v14 because we forget to update the ci to test for it. Testing v12 does not mean v14 will always work.

[calculuschild]

Babel routinely drops support for older platforms including Node as they age out, and garuntees that our code will work on supported versions of Node. We wouldn't even have to think about it as long as we keep Babel up to date.

... right?

[UziTech]

Than we would have to make sure we use a version of babel that supports the versions of node that we support. That seems like more work than maintaining ci to test all versions we support.

[calculuschild]

we would have to make sure we use a version of babel that supports the versions of node that we support

If we already agree that we only support active versions of Node, then keeping Babel up to date does that for us. There will be no need to have CI testing for different Node versions since Babel already garuntees our code will work.

Babel only seems to drop support for node versions with a major version bump, so we can even keep an eye out for that and update our own version accordingly if we really want to.

"We support active versions of Node.js" == "We support whatever the latest version of Babel supports."

[UziTech]

That could work.

I do recall @styfle sharing this article a while ago which, if I remember correctly, argues that we should only support the latest version of node and let whoever wants to maintain marked for other versions do it themselves (similar to what I have been saying).

Not that I agree with it 100% but there are some good points.

[styfle]

I don’t remember sharing that article 🤔

I’m still in favor of bumping semver major when dropping a version.

[UziTech]

I don’t remember sharing that article 🤔

It was in a team discussion in 2018

I’m still in favor of bumping semver major when dropping a version.

I agree but I think we need to determine what constitutes "dropping" a version. Currently marked v2.1.1 still works on all the same node versions as marked v2.0.7. The only difference is the engines field. If we are going by what is tested than v2.0.7 only supported v14 and v16 and 2.1.1 supports v12, v14, and v16 so support was added not dropped.

[styfle]

Then it sounds like we need to change the engines field 🙂

[UziTech]

Then it sounds like we need to change the engines field 🙂

Right, but to what?

Do we go with accuracy? What is the minimum version that marked works on? Does it work on every version after that? or is there some bug in node that prevents it from working on some versions?

Do we set it back to what it was at before v2.1.1 even though it was inaccurate?

Do we go with an arbitrary value like v12?

I don't think any person wants to do the work to make sure it is accurate which is why I think we should just remove it.

[styfle]

How about PR #2119?

[styfle]

Regarding the article, I think we're reading it differently. The way I read it was essentially this quote:

You, in your GitHub project, probably don’t have the time or resources to manage more than one release line of development.

Meaning, if you ship 2.1.1 and then you ship 3.0.0, you don't ever need to ship 2.1.2 because that release line is no longer maintained.

[UziTech]

stop supporting use cases for which nobody concerned with that case is willing to contribute.

I think this is the part of the article that sums up my arguments the best.

[UziTech]

Or this:

Saying you won’t do the work to support an old release isn’t saying “I don’t care about you being a user.” Instead, you’re saying that there are two ways they can be a user.

• Upgrade.
• Help me keep it running for people who won’t upgrade.

[Fleker]

Personally I completely agree it wouldn't be fair or reasonable to expect work to support my increasingly outdated version of Node. If you decided to stop development on v2 and put effort exclusively into v3, I would not mind. But if you drop support for a particular version of Node, I would prefer that to be communicated as a major version to prevent users from being surprised.

[github-actions]

🎉 This issue has been resolved in version 2.1.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀