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

Rename the children prop #887

Closed
4 tasks done
remcohaszing opened this issue Feb 6, 2025 · 5 comments
Closed
4 tasks done

Rename the children prop #887

remcohaszing opened this issue Feb 6, 2025 · 5 comments
Labels
🙅 no/wontfix This is not (enough of) an issue for this project 👎 phase/no Post cannot or will not be acted on

Comments

@remcohaszing
Copy link
Member

Initial checklist

Problem

We require children to be a string. In theory this is completely fine, but sometimes this is confusing for users or doesn’t play nice with external tooling. Users often don’t know what they pass as children. Many users already struggle with multiline strings, but JSX children add another layer of confusion. #886 is a recent example. Typically children are passed to React directly, but in our case it’s processed using a unified processor.

Some examples I could come up with:

<Markdown>
  # Hello

  world
</Markdown>
<Markdown>
  # Hello

  <br />

  world
</Markdown>
<Markdown>
  # Hello

  world
</Markdown>
<Markdown>{`
  # Hello

  world
`}</Markdown>

Current solutions

Tell users the problem is outside of react-markdown. (Which it is! But we can improve react-markdown anyway.)

Proposed solutions

Rename the children prop. For example, name it content, value, or markdown.

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Feb 6, 2025
@wooorm
Copy link
Member

wooorm commented Feb 6, 2025

I am against this. There is no need to cause a breaking change for tiny gains. Especially if it’s a trade off that also comes with losses.

If we rename it to value then people will still pass weird things—numbers, arrays.
I also wager that more people will be confused that children (the prop or actual ones) don’t work.

A couple years ago, the property was renamed from value to children. There are also arguments for this. One of them is consistency with other tools such as https://github.com/quantizor/markdown-to-jsx.

@wooorm
Copy link
Member

wooorm commented Feb 12, 2025

@remcohaszing Shall I then close this?

@remcohaszing
Copy link
Member Author

It’s a prop rename. We’ve had more of those. I also don’t think we should cut a major release just for this, but when it’s time for a major release, I would welcome this change.

I also believe people will still pass invalid things if the prop is named differently. Indented multiline strings come to mind. But the children prop invites user to pass more categories of invalid things, such as JSX elements or expressions.

I wouldn’t prioritize similarity to other libraries. I don’t really see the benefit of using the children prop.

This is just a suggestion though. The current situation works on a technical level. Feel free to close this issue. But I think it would be better to avoid the children prop for this use case.

@wooorm
Copy link
Member

wooorm commented Feb 12, 2025

Sure we’ve had breaking changes, but we don’t need to have breaking changes.
There are a million users of this package and you propose that they all spend 5 minutes to change their code bases: a colossal cost!
When there is the inevitable major, we don’t need to change this.

@wooorm wooorm closed this as not planned Won't fix, can't repro, duplicate, stale Feb 12, 2025
@wooorm wooorm added the 🙅 no/wontfix This is not (enough of) an issue for this project label Feb 12, 2025

This comment has been minimized.

@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙅 no/wontfix This is not (enough of) an issue for this project 👎 phase/no Post cannot or will not be acted on
Development

No branches or pull requests

2 participants