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

Erroneous warnings about deprecated options when using parse() #2793

Closed
Conduitry opened this issue May 2, 2023 · 33 comments · Fixed by #2890
Closed

Erroneous warnings about deprecated options when using parse() #2793

Conduitry opened this issue May 2, 2023 · 33 comments · Fixed by #2890

Comments

@Conduitry
Copy link

Marked version:
5.0.0

Describe the bug
Calling parse emits erroneous warnings about deprecated options that are not actually being used.

To Reproduce

  1. Install marked@5.0.0
  2. Run require('marked').parse('foo') in REPL
  3. Observe warnings

Expected behavior
The warnings should only be emitted for deprecated options that I am using.

@austenc
Copy link

austenc commented May 3, 2023

Also seeing this on a fresh install of 5.0 -- I'm new to using marked, but it seems like a bug.

@UziTech
Copy link
Member

UziTech commented May 3, 2023

Those warnings are not erroneous. Some options that are being deprecated are true by default. You can silence them by changing the options with:

marked.use({
  langPrefix: '',
  mangle: false,
  headerIds: false
});

@Conduitry
Copy link
Author

I see. I'd suggest a different warning message then that's more explicit about what's going on.

@UziTech
Copy link
Member

UziTech commented May 3, 2023

PRs are always welcome 😁👍

@mateuscruz
Copy link

@UziTech I have created a PR to suppress these warnings when no opt is passed to marked.

@terrablue
Copy link

Not really sure what's the idea here. Warning for implicitly used stuff, forcing users to explicitly deactivate options that will be removed in the future or alternatively turn off warnings altogether? Neither seems like a good idea to me.

Wouldn't it be better to remove these implicit opt-in warnings and just break it in a future major and make a note of it?

@calculuschild
Copy link
Contributor

calculuschild commented May 3, 2023

I agree with @UziTech. This is pretty standard practice for projects moving to deprecate features (whether implicit or not). Take a look at major industry projects like Mongoose or React that do this all the time. We can understand it's disruptive, but this is not at all unusual.

When a default feature is being renamed or split out into its own package or otherwise deprecated, it's extremely common to provide a heads-up (yes, as a warning in the console) even if it isn't going away until the next major version. This is simply providing a warning so you can make any changes before the next version breaks your product.

@quinton-ashley
Copy link

quinton-ashley commented May 3, 2023

Totally absurd amount of warnings. I agree with @terrablue. The average user doesn't care about setting implicit options to false. No idea what y'all are thinking. I'll be sticking to version 4.3.0 I guess.

@mateuscruz
Copy link

This is what I've done on my codebase (I use Typescript):

export const markdownToHtml = (markdownSource: string): string => marked(
  markdownSource,
  {
    langPrefix: undefined,
    mangle: false,
    headerIds: false
  }
)

In my case, this is the only reference to marked as I use wrapper functions for most dependencies. If you use marked directly everywhere (you may want to read this), going with marked.use, as recommended by @UziTech is the way to go for now.

@UziTech
Copy link
Member

UziTech commented May 4, 2023

#2796 will remove one of the default warnings so you will be able to use the following to silence the default warnings:

marked.use({
  mangle: false,
  headerIds: false
});

@LitileXueZha
Copy link

LitileXueZha commented May 4, 2023

@UziTech May I ask why marked is split to bunch of small packages alone?

At past , it's out-of-box and only install one marked, now or in furture, we need to install marked and marked-*. To tell the truth, I'm tired to find these packages one by one unless it's really needed.

@UziTech
Copy link
Member

UziTech commented May 4, 2023

TL;DR We want to prevent and remove feature creep.

Over the years features have been added that have nothing to do with converting markdown to html according to the GFM and CommonMark spec. These are great features, but every features we add(ed) requires more mantainance costs and make updating marked more difficult.

Over the last couple years we have been adding ways to extend marked so these great features can be maintained separately. We are finally in a place where these features can be moved out of marked.

This will also reduce the footprint of marked. For most of the users that don't need these features there is no reason to have the code in marked.

If you just want to convert markdown to html you will still be able to just install marked. It is only when you want these extra features that you will need to find or create an extension.

@quinton-ashley
Copy link

quinton-ashley commented May 4, 2023

I agree with @LitileXueZha marked being an all in one, no setup required, easy to use package was its biggest asset.

@UziTech marked wasn't even that big. If adding new features made upgrading marked more difficult for you, that's a problem that you as a developer need to figure out how to solve, not some extra work you should force on people using marked. I think you've underestimated how many people want marked to include stuff like code highlighting, which should not be an "advanced" feature.

You've ruined the main appeal of marked!

@UziTech
Copy link
Member

UziTech commented May 4, 2023

marked wasn't even that big.

That is because we didn't allow all of the features that anyone wanted to be included. It is still bigger than it needs to be.

If adding new features made upgrading marked more difficult for you, that's a problem that you as a developer need to figure out how to solve

That is exactly what we are doing.

not some extra work you should force on people using marked.

I don't know if you realize this, but no one who maintains marked gets paid for it. That means any additional work we need to do is for free. Pushing work on users that want to use free software is the way we are able to continue maintaining it and still give users the features they want.

I think you've underestimated how many people want marked to include stuff like code highlighting

Do you have data to back this up? I think you may be over estimating how many people want marked to include highlighting. This way it is still available with a little extra work.

@austenc
Copy link

austenc commented May 4, 2023

It definitely makes sense to have it split up into separate packages. Especially with ESM support (which it has).

Thanks for the time and effort on marked -- it works great! Thanks for the info about the warnings being notices of future deprecation. That makes them less confusing. Cheers!

@quinton-ashley
Copy link

quinton-ashley commented May 4, 2023

People just want to use marked.parse and have code highlighting included, without having to add it in as an extra addon and also configure a bunch of options to false when they have no idea what the options even are.

Look at the like/dislike ratio on all your comments vs what other people posted here to see how wrong you are. Literally only one person unaffiliated with marked has said they agree with you.

@DanielSchiavini
Copy link

DanielSchiavini commented May 4, 2023

Thanks for all the work maintaining this.
However it would be better to add a warning to the changelog only. People should read anyway when upgrating major versions.
The warn messages were very confusing because I'm literally using no options whatsoever.

@quinton-ashley
Copy link

@UziTech also you said "PRs are always welcome" and then someone took the time to fix your mistakes and submitted a PR, but you closed it. 🤡

@calculuschild
Copy link
Contributor

@quinton-ashley Cool the attitude. You can disagree without the hostility.

Look at the like/dislike ratio on all your comments...

Come on. We can see you're the only one doing this. It doesn't help.

Projects evolve and get refactored all the time. We understand that it adds some extra burden on users, but the change was not made just to spite you. There is a reason this update was made under a major version bump, because we knew it would be disruptive. Users are always free to stick with an older version if this does not suit them.

In the mean time there are already some adjustments being made based on the feedback, so thanks to all of you for your comments and support. Let's try to keep the temperature down on this thread. There's no need for aggression or personal attacks.

@quinton-ashley
Copy link

quinton-ashley commented May 4, 2023

I'm not the only one lol. The original post saying the warnings are erroneous got 4 upvotes. There's like 8 other people saying you're wrong for various reasons. If you're not going to fix it, even when people did the work for you in a PR, the only thing people can do is complain about how stupid these changes are. Not everyone is going to be polite about it.

I almost pushed to production with like 50 nonsense warnings from marked in the console of every page. Like how the fuck did you expect people to react to that? @UziTech

The longer you keep v5 as it is the more people will complain. It's only been 2 days since you released the changes. Hasn't the response been bad enough already? I'd be so embarrassed if I were you guys.

@UziTech
Copy link
Member

UziTech commented May 4, 2023

@UziTech also you said "PRs are always welcome" and then someone took the time to fix your mistakes and submitted a PR, but you closed it

I didn't close it, they did. If you are going to use information to back up your arguments you may want to double check that your information is accurate.

@quinton-ashley
Copy link

quinton-ashley commented May 4, 2023

@UziTech You're nitpicking. You still said "PRs are welcome 😁👍" and then when some guy actually took the time to make a PR you immediately responded with "We still want to alert people that the options are deprecated and will change in the future if they are left as the default." 🤡

No one else actually wants to be forced to set those options if they didn't even set any in the first place.

@mateuscruz
Copy link

mateuscruz commented May 6, 2023

I was the one who closed the PR. And I did it because I understand the reasons the maintainers have for these warnings, even though I disagree with them. I do believe there are other ways to tackle this, that would be less disruptive.

What I would've done is:

  • add the deprecation warnings to the Changelog,
  • add a quick migration guide for people actually using those custom options (which would be the opposite of the solution being suggested here)
  • and then only show the warnings to people using the custom options. This could be achieved by breaking down marked.defaults logic into two separate objects: a preset list of properties where everything would be disabled by default, and the actual defaults constant whose initial value would be copied from preset, but could be overridden by marked.use.

I believe this would've been less disruptive to the community overall.

Then the migrate instructions would go somewhat along these lines:

// remove the opts you don't use
marked.use({
  langPrefix: 'language-',
  mangle: true,
  headerIds: true,
  headerPrefix: 'yourHeaderPrefix'
});

There would be no need to add highlight to the list because people coming from v4 would already have it set anyway (the default value in v4 was already null)

But the maintainers seem to disagree with me on that and that's ok, it's just my opinion. We're grownups.
I closed the PR because I don't have the time to work on or discuss an alternate solution right now, as I'm swamped with work. I'm not a maintainer of this project, I just thought it would be a quick solution and was willing to propose it but it isn't.

It's not the end of the world, there is a way to silence the warnings, after all. Not as convenient as we would like, but it exists.

There's no reason to be rude about it.

@mateuscruz
Copy link

@UziTech let me know if you think my suggestion makes sense and I can open a new PR for that.

@mateuscruz
Copy link

mateuscruz commented May 6, 2023

BTW, for those who are using custom options that would like to suppress the warnings, this works:

marked.use({ silent: true });

@UziTech
Copy link
Member

UziTech commented May 6, 2023

@mateuscruz I don't understand how the presets would help. I agree with having a migration plan, but we still don't want to change the defaults yet and we want to warn people who want the headerids and mangle to use the extensions or turn them off.

@steffen-4s1
Copy link

Could you please update your release notes for v5.0.1 from

marked -no-mangle -no-header-ids ...

to

marked --no-mangle --no-header-ids ...

Otherwise the parameters will not work under Linux and the deprecation warnings will appear.

@UziTech
Copy link
Member

UziTech commented May 17, 2023

@steffen-4s1 thanks for letting us know. I fixed it

@yoonghm
Copy link

yoonghm commented Jun 24, 2023

I have installed marked-mangle, marked-gfm-heading-id but warning still appear. How should I disable these warning?

$ marked -i python.md -o python.html
marked(): mangle parameter is enabled by default, but is deprecated since version 5.0.0, and will be removed in the future. To clear this warning, install https://www.npmjs.com/package/marked-mangle, or disable by setting `{mangle: false}`.
marked(): headerIds and headerPrefix parameters enabled by default, but are deprecated since version 5.0.0, and will be removed in the future. To clear this warning, install  https://www.npmjs.com/package/marked-gfm-heading-id, or disable by setting `{headerIds: false}`.
$ marked --no-mangle --no-header-ids -i python.md -o python.html

There was no error for the last command. I think marked has to verify that if those packages are installed before putting those messages.

@UziTech
Copy link
Member

UziTech commented Jun 24, 2023

You have to use them as an extension to remove the warning. Currently there is no way to use extensions with the cli.

@kruncher
Copy link

Why does the marked CLI display these errors? perhaps it would be better to jump a major version number and change the defaults. Updating to a major version implies that there might be more to the update.

@UziTech
Copy link
Member

UziTech commented Jul 17, 2023

We did. We introduced these warnings in v5.0.0

@UziTech
Copy link
Member

UziTech commented Jul 17, 2023

We will update the defaults so these warnings don't show by default in v6. We are still waiting on some things for the v6 release but hopefully it will be soon.

diogokiss added a commit to diogokiss/semantic-release that referenced this issue Sep 4, 2023
This commit disables to flags that are deprecated in the 'marked-terminal' as
pointed in the following issue.

markedjs/marked#2793

```
marked(): mangle parameter is enabled by default, but is deprecated since
version 5.0.0, and will be removed in the future. To clear this warning, install
https://www.npmjs.com/package/marked-mangle, or disable by setting
`{mangle: false}`.

marked(): headerIds and headerPrefix parameters enabled by default, but are
deprecated since version 5.0.0, and will be removed in the future. To clear this
warning, install  https://www.npmjs.com/package/marked-gfm-heading-id, or
disable by setting `{headerIds: false}`.
```
diogokiss added a commit to diogokiss/semantic-release that referenced this issue Sep 4, 2023
This commit disables to flags that are deprecated in the 'marked-terminal' as
pointed in the following issue.

markedjs/marked#2793

```
marked(): mangle parameter is enabled by default, but is deprecated since
version 5.0.0, and will be removed in the future. To clear this warning, install
https://www.npmjs.com/package/marked-mangle, or disable by setting
`{mangle: false}`.

marked(): headerIds and headerPrefix parameters enabled by default, but are
deprecated since version 5.0.0, and will be removed in the future. To clear this
warning, install  https://www.npmjs.com/package/marked-gfm-heading-id, or
disable by setting `{headerIds: false}`.
```
diogokiss added a commit to diogokiss/semantic-release that referenced this issue Sep 4, 2023
This commit disables to flags that are deprecated in the 'marked-terminal' as
pointed in the following issue.

markedjs/marked#2793

```
marked(): mangle parameter is enabled by default, but is deprecated since
version 5.0.0, and will be removed in the future. To clear this warning, install
https://www.npmjs.com/package/marked-mangle, or disable by setting
`{mangle: false}`.

marked(): headerIds and headerPrefix parameters enabled by default, but are
deprecated since version 5.0.0, and will be removed in the future. To clear this
warning, install  https://www.npmjs.com/package/marked-gfm-heading-id, or
disable by setting `{headerIds: false}`.
```
Xunnamius added a commit to Xunnamius/next-test-api-route-handler that referenced this issue Nov 4, 2023
…gelog

Specifically, the decision to start dumping warning text into perfectly good
output text in a non-major release of a deep dependency (marked) is kind of
effed up. More information here: markedjs/marked#2793
diogokiss added a commit to diogokiss/semantic-release that referenced this issue Feb 12, 2024
This commit disables to flags that are deprecated in the 'marked-terminal' as
pointed in the following issue.

markedjs/marked#2793

```
marked(): mangle parameter is enabled by default, but is deprecated since
version 5.0.0, and will be removed in the future. To clear this warning, install
https://www.npmjs.com/package/marked-mangle, or disable by setting
`{mangle: false}`.

marked(): headerIds and headerPrefix parameters enabled by default, but are
deprecated since version 5.0.0, and will be removed in the future. To clear this
warning, install  https://www.npmjs.com/package/marked-gfm-heading-id, or
disable by setting `{headerIds: false}`.
```
diogokiss added a commit to diogokiss/semantic-release that referenced this issue Jun 3, 2024
This commit disables to flags that are deprecated in the 'marked-terminal' as
pointed in the following issue.

markedjs/marked#2793

```
marked(): mangle parameter is enabled by default, but is deprecated since
version 5.0.0, and will be removed in the future. To clear this warning, install
https://www.npmjs.com/package/marked-mangle, or disable by setting
`{mangle: false}`.

marked(): headerIds and headerPrefix parameters enabled by default, but are
deprecated since version 5.0.0, and will be removed in the future. To clear this
warning, install  https://www.npmjs.com/package/marked-gfm-heading-id, or
disable by setting `{headerIds: false}`.
```
diogokiss added a commit to diogokiss/semantic-release that referenced this issue Oct 7, 2024
This commit disables to flags that are deprecated in the 'marked-terminal' as
pointed in the following issue.

markedjs/marked#2793

```
marked(): mangle parameter is enabled by default, but is deprecated since
version 5.0.0, and will be removed in the future. To clear this warning, install
https://www.npmjs.com/package/marked-mangle, or disable by setting
`{mangle: false}`.

marked(): headerIds and headerPrefix parameters enabled by default, but are
deprecated since version 5.0.0, and will be removed in the future. To clear this
warning, install  https://www.npmjs.com/package/marked-gfm-heading-id, or
disable by setting `{headerIds: false}`.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet