Skip to content

Conversation

@pastacolsugo
Copy link

@pastacolsugo pastacolsugo commented Apr 16, 2025

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and discussions and couldn’t find anything or linked relevant results below
  • I made sure the docs are up to date
  • I included tests (or that’s not needed)

Description of changes

Hello everyone,
thank you for your work on react-markdown!

I'm submitting this PR to propose a Developer Experience improvement, by removing the default export in index.js.

The current code exports the Markdown component (synchronous version) as default.

This default export makes it possible to import the module by doing:

import Markdown from 'react-markdown';

In my case I then realised I needed MarkdownHooks, thus changed my import as usual, to:

import MarkdownHooks from 'react-markdown';

And here lies the catch: I'm still importing the Markdown component!

I forgot to add curly braces, and the default export happily worked to import the Markdown in the confusingly named MarkdownHooks variable.

Removing the default export would force the developer to import using curly braces from the beginning, avoiding this confusing mistake. It's also consistent with how most other packages work.

This PR removes the default export, and updates the docs accordingly.

This article explains other benefits of removing default exports better than I could.

Cheers! 🎲

@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 Apr 16, 2025
@ChristianMurphy ChristianMurphy added 🗄 area/interface This affects the public interface 🗳 blocked/vote This cannot progress before voting is complete 🧑 semver/major This is a change 💬 type/discussion This is a request for comments labels Apr 16, 2025
@remcohaszing
Copy link
Member

I also prefer named exports. But this is definitely a breaking change. I don’t think we should release a semver major just for this.

Missing `Markdown` component reference

Signed-off-by: Ugo Baroncini <[email protected]>
@pastacolsugo
Copy link
Author

That's a fair concern.
It's a shame it wasn't introduced with the latest major, which was less than two months ago.
Do you plan on making another major release in the next months?

The decision is not mine to make, but you could consider that two months since the latest major is a small window of time: I would say that most people still haven't updated to it.
Interestingly the npm page for the package shows exactly that, with most people still being on version 9!
I would also assume that a bunch of those v10 downloads are active development (like I'm doing!), where this change would be quickly fixed.
In addition to that, only those importing it via the default would be affected, which (I would assume) are not most.

Do you see another major in the next months? If not, I would consider doing it now and being done with it, instead of dragging it.

:)

@wooorm
Copy link
Member

wooorm commented Apr 16, 2025

hello! I would agree with you if there were few users.

I am very strongly against this. You are asking 1 234 567 people to change their code. For no real benefit. Also, there are 10 years of examples out there: LLMs will keep outputting the old version that doesn’t work for years. As you show in your use case, many users of this package are beginners and will mix things up.

backcompat is nice. We don’t have to break things. Sometimes the world isn’t perfect, that’s ok

@wooorm wooorm closed this Apr 16, 2025
@github-actions
Copy link

Hi! This was closed. Team: If this was merged, please describe when this is likely to be released. Otherwise, please add one of the no/* labels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🗄 area/interface This affects the public interface 🗳 blocked/vote This cannot progress before voting is complete 🤞 phase/open Post is being triaged manually 🧑 semver/major This is a change 💬 type/discussion This is a request for comments

Development

Successfully merging this pull request may close these issues.

4 participants