Fix #1823 - task lists are rendered even when GFM is disabled

[TheCloudlessSky]

Marked version:

1.2.3

Markdown flavor:

All except GitHub Flavored Markdown

Description

• Fixes Task lists are rendered even when GFM is disabled #1823

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.

• CI is green (no forced merge required).
• Squash and Merge PR following conventional commit guidelines.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/markedjs/markedjs/j6p2afjvu
✅ Preview: https://markedjs-git-tasklist-bug.markedjs.vercel.app

[UziTech]

LGTM 💯

[styfle]

Looks good, thanks!

However, this bug has existed for at least a year so we might need to consider it a breaking change for users that have come to rely on this behavior.

[TheCloudlessSky]

However, this bug has existed for at least a year so we might need to consider it a breaking change for users that have come to rely on this behavior.

Agree. Is marked ever going to use semantic versioning? Semi-related: I recently upgraded an old version of Marked and it broke even the simplest of Markdown that was supported a while ago with GFM: **Hello ** World would render <strong>Hello </strong> World and now it renders literally **Hello ** World. I found the small breaking-change comments in the release notes was not a strong/obvious enough signal that things really broke vs a major version bump.

[styfle]

Is marked ever going to use semantic versioning

It already does follow semver, we just have to agree on what is considered a patched bug fix or is breaking.

Nearly every change to marked is going to affect the resulting output so we could consider every change as major semver (besides something like an internal refactor).

However, since we document that we follow the Commonmark/GFM spec, then any change to align with that spec can be considered a patch even though the output changed.

In this case, since its not a fix to the spec but actually a change to the behavior to the public API, I consider this a major semver change.

[UziTech]

However, this bug has existed for at least a year so we might need to consider it a breaking change for users that have come to rely on this behavior.

I disagree. First since gfm is true by default most users won't be affected, and as @TheCloudlessSky pointed out we would have to change the major version on every fix just incase someone is relying on it.

This is a FIX and should be a patch.

[UziTech]

since its not a fix to the spec

This is a fix to the spec.

Before this fix the following markdown did not match CommonMark:

Marked demo

CommonMark demo

- [ ] A
- [ ] B
- [ ] C

[calculuschild]

I recently upgraded an old version of Marked and it broke even the simplest of Markdown that was supported a while ago with GFM

For what it's worth, this was another fix that was made to align more closely to the Markdown spec. The extra spaces in **hello ** are invalid markdown and should be rendered as asterisks instead of <strong>. So it's not so much that Marked "dropped support" for a feature, but rather patched out a bug that was incorrectly generating HTML elements that it shouldn't.

It is true that the presence of bugs sometimes means people come to expect a certain behavior, but as @UziTech says, I don't think a patch to fix those bugs requires a major version bump, even if people have unknowingly come to rely on them.

[UziTech]

#1834 was just merged to clarify our versioning strategy. https://marked.js.org/publishing#versioning

I think since this PR moves Marked closer to CommonMark compliance and does not touch our public API it should be a patch update.

[UziTech]

If this can get one more approval we can merge it

/cc @joshbruce @calculuschild @styfle @davisjam

[github-actions]

🎉 This PR is included in version 1.2.5 🎉

The release is available on:

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

Your semantic-release bot 📦🚀