Clarify deprecation messages for default parameters #2802

calculuschild · 2023-05-08T17:27:19Z

Marked version: 5.0.1

Markdown flavor: n/a

Description

Fixes CLI deprecation warning #2797

Adds a few clarifying words to the deprecation messages for those parameters that are enabled by default. Users are confused about why they are receiving warnings for parameters they didn't manually set; this help indicate that they are enabled by default and they can also just disable the setting if they don't want to get the extension.

I could see changing "historically" to "currently" (or just removing), if that is more clear wording.

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 · 2023-05-08T17:27:23Z

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	May 11, 2023 2:44pm

styfle

Users are confused about why they are receiving warnings for parameters they didn't manually set

Isn't that a bug? I think we should check if it was assigned before warning.

UziTech · 2023-05-09T01:40:51Z

We want to alert people who don't set it but still want it that it is deprecated and let them know they should use the extension.

I'm not sure how much time should pass before we change the default to false for those settings but it should probably be soon. This will be a breaking change of course so we will need to release marked v6

styfle · 2023-05-09T21:32:36Z

The message is still confusing to me.

The beginning "should not be used and will be removed in the future" seems contradictory to he second part "can be manually disabled".

I think anyone getting this warning today is going to immediately set {mangle: false, headerIds: false} to silence it, so we are basically increasing usage 😅

Maybe we should create an upgrade guide and have the warning print a link to it.

calculuschild · 2023-05-09T21:36:40Z

The beginning "should not be used and will be removed in the future" seems contradictory to he second part "can be manually disabled".

I'm not sure I understand your point here. Mangle by default is "true". However, is is being deprecated, so users need to manually disable it (or use the separate extension) if they want the deprecation message to go away.

I think anyone getting this warning today is going to immediately set {mangle: false, headerIds: false} to silence it, so we are basically increasing usage 😅

No, setting {mangle: false} would decrease usage of Mangle, which is what we want.

styfle · 2023-05-09T21:43:43Z

(or use the separate extension) if they want the deprecation message to go away.

Does using the extension automatically disable it?

calculuschild · 2023-05-09T21:46:41Z

Ah, probably not. I guess they need to disable the Mangle option either way to get rid of the warning. Then if they still want the mangle functionality, they should get the extension.

styfle

I see. That's what I thought. So the message is not very clear. Let's update the message to make this clear 👍

styfle · 2023-05-09T22:16:46Z

src/helpers.js

@@ -250,7 +250,7 @@ export function checkDeprecations(opt, callback) {
  }

  if (opt.mangle) {
-    console.warn('marked(): mangle parameter is deprecated since version 5.0.0, should not be used and will be removed in the future. Instead use https://www.npmjs.com/package/marked-mangle.');
+    console.warn('marked(): mangle parameter is deprecated since version 5.0.0, should not be used and will be removed in the future. Note the "mangle" parameter is historically enabled by default, but can be manually disabled, or instead use https://www.npmjs.com/package/marked-mangle.');


Suggested change

console.warn('marked(): mangle parameter is deprecated since version 5.0.0, should not be used and will be removed in the future. Note the "mangle" parameter is historically enabled by default, but can be manually disabled, or instead use https://www.npmjs.com/package/marked-mangle.');

console.warn('marked(): mangle parameter is deprecated since version 5.0.0, and will be removed in the future. To enable, install https://npmjs.com/marked-mangle. To disable, set `{mangle: false}`.');

styfle · 2023-05-09T22:17:42Z

src/helpers.js

@@ -266,7 +266,7 @@ export function checkDeprecations(opt, callback) {
  }

  if (opt.headerIds || opt.headerPrefix) {
-    console.warn('marked(): headerIds and headerPrefix parameters are deprecated since version 5.0.0, should not be used and will be removed in the future. Instead use https://www.npmjs.com/package/marked-gfm-heading-id.');
+    console.warn('marked(): headerIds and headerPrefix parameters are deprecated since version 5.0.0, should not be used and will be removed in the future. Note the "headerIds" parameter is historically enabled by default, but can be manually disabled, or instead use https://www.npmjs.com/package/marked-gfm-heading-id.');


Suggested change

console.warn('marked(): headerIds and headerPrefix parameters are deprecated since version 5.0.0, should not be used and will be removed in the future. Note the "headerIds" parameter is historically enabled by default, but can be manually disabled, or instead use https://www.npmjs.com/package/marked-gfm-heading-id.');

console.warn('marked(): headerIds and headerPrefix parameters are deprecated since version 5.0.0, and will be removed in the future. To enable, install https://npmjs.com/marked-gfm-heading-id. To disable, set `{headerIds: false}`.');

UziTech · 2023-05-09T23:37:27Z

The mangle extension does disable it.
https://github.com/markedjs/marked-mangle/blob/1dffd6f4bb7a1dd7852d6aece93c79982f710812/src/index.js#L3

UziTech · 2023-05-09T23:44:58Z

The header id extension doesn't but I will fix that tonight.

UziTech · 2023-05-10T02:07:35Z

I fixed the header-ids extension. 👍

calculuschild · 2023-05-10T02:44:45Z

Alright. So if the extension already disables the option, is the wording clear as-is, @styfle? Or does that influence your suggested change?

styfle · 2023-05-11T02:29:27Z

@calculuschild I updated my suggestions

calculuschild · 2023-05-11T02:57:40Z

My main goal with this PR was to alert users that these options are enabled by default, to address the confusion of "Users are confused about why they are receiving warnings for parameters they didn't manually set;" So something to that effect needs to remain in the message.

I will take your suggestions, @styfle, and tweak to reintroduce the mention of "enabled by default" so this can still solve the issue it was intended to.

styfle · 2023-05-11T13:58:12Z

We need to make sure the message is actionable.

Alerting for the sake of alerting is going to cause more pain for users.

calculuschild · 2023-05-11T14:18:01Z

Right. Actionable, but also explain why the action is needed, which is the root of the user complaints. We can do both.

Edit: @styfle , I have rewritten the messages to include both the explicit {mangle: false} instruction, as well as the explanation that this parameter is enabled by default.

styfle

Perfect, thanks! 🥇

## [5.0.2](v5.0.1...v5.0.2) (2023-05-11) ### Bug Fixes * Clarify deprecation messages for default parameters ([#2802](#2802)) ([763e9de](763e9de))

Clarify deprecation messages for default parameters

4cb102d

vercel bot deployed to Preview May 8, 2023 17:27 View deployment

calculuschild requested review from UziTech, styfle, joshbruce and davisjam May 8, 2023 17:29

UziTech approved these changes May 8, 2023

View reviewed changes

styfle reviewed May 8, 2023

View reviewed changes

styfle requested changes May 9, 2023

View reviewed changes

Tweak actionable steps to be more explicit.

36e04f9

vercel bot deployed to Preview May 11, 2023 14:44 View deployment

calculuschild requested review from styfle and UziTech May 11, 2023 14:44

UziTech approved these changes May 11, 2023

View reviewed changes

styfle approved these changes May 11, 2023

View reviewed changes

UziTech merged commit 763e9de into markedjs:master May 11, 2023
11 checks passed

github-actions bot pushed a commit that referenced this pull request May 11, 2023

chore(release): 5.0.2 [skip ci]

f019379

## [5.0.2](v5.0.1...v5.0.2) (2023-05-11) ### Bug Fixes * Clarify deprecation messages for default parameters ([#2802](#2802)) ([763e9de](763e9de))

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clarify deprecation messages for default parameters #2802

Clarify deprecation messages for default parameters #2802

calculuschild commented May 8, 2023 •

edited

vercel bot commented May 8, 2023 •

edited

styfle left a comment

UziTech commented May 9, 2023

styfle commented May 9, 2023

calculuschild commented May 9, 2023 •

edited

styfle commented May 9, 2023

calculuschild commented May 9, 2023

styfle left a comment

styfle May 9, 2023 •

edited

styfle May 9, 2023 •

edited

UziTech commented May 9, 2023 •

edited

UziTech commented May 9, 2023

UziTech commented May 10, 2023

calculuschild commented May 10, 2023 •

edited

styfle commented May 11, 2023

calculuschild commented May 11, 2023 •

edited

styfle commented May 11, 2023

calculuschild commented May 11, 2023 •

edited

styfle left a comment

	console.warn('marked(): mangle parameter is deprecated since version 5.0.0, should not be used and will be removed in the future. Note the "mangle" parameter is historically enabled by default, but can be manually disabled, or instead use https://www.npmjs.com/package/marked-mangle.');
	console.warn('marked(): mangle parameter is deprecated since version 5.0.0, and will be removed in the future. To enable, install https://npmjs.com/marked-mangle. To disable, set `{mangle: false}`.');

	console.warn('marked(): headerIds and headerPrefix parameters are deprecated since version 5.0.0, should not be used and will be removed in the future. Note the "headerIds" parameter is historically enabled by default, but can be manually disabled, or instead use https://www.npmjs.com/package/marked-gfm-heading-id.');
	console.warn('marked(): headerIds and headerPrefix parameters are deprecated since version 5.0.0, and will be removed in the future. To enable, install https://npmjs.com/marked-gfm-heading-id. To disable, set `{headerIds: false}`.');

Clarify deprecation messages for default parameters #2802

Clarify deprecation messages for default parameters #2802

Conversation

calculuschild commented May 8, 2023 • edited

Description

Contributor

Committer

vercel bot commented May 8, 2023 • edited

styfle left a comment

Choose a reason for hiding this comment

UziTech commented May 9, 2023

styfle commented May 9, 2023

calculuschild commented May 9, 2023 • edited

styfle commented May 9, 2023

calculuschild commented May 9, 2023

styfle left a comment

Choose a reason for hiding this comment

styfle May 9, 2023 • edited

Choose a reason for hiding this comment

styfle May 9, 2023 • edited

Choose a reason for hiding this comment

UziTech commented May 9, 2023 • edited

UziTech commented May 9, 2023

UziTech commented May 10, 2023

calculuschild commented May 10, 2023 • edited

styfle commented May 11, 2023

calculuschild commented May 11, 2023 • edited

styfle commented May 11, 2023

calculuschild commented May 11, 2023 • edited

styfle left a comment

Choose a reason for hiding this comment

calculuschild commented May 8, 2023 •

edited

vercel bot commented May 8, 2023 •

edited

calculuschild commented May 9, 2023 •

edited

styfle May 9, 2023 •

edited

styfle May 9, 2023 •

edited

UziTech commented May 9, 2023 •

edited

calculuschild commented May 10, 2023 •

edited

calculuschild commented May 11, 2023 •

edited

calculuschild commented May 11, 2023 •

edited