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

No way to know if a component is JSX or MDX in MDXProvider #560

Closed
danoc opened this issue Apr 30, 2019 · 9 comments
Closed

No way to know if a component is JSX or MDX in MDXProvider #560

danoc opened this issue Apr 30, 2019 · 9 comments
Labels
💪 phase/solved Post is done 🦋 type/enhancement This is great to have 💎 v2 Issues related to v2

Comments

@danoc
Copy link

danoc commented Apr 30, 2019

Version 1.0 includes a change:

The pragma implementation will also cause JSX HTML elements to be rendered with the component mapping passed to MDXProvider.

It seems to be difficult (or impossible?) to opt-out of this behavior and render the JSX differently than the MDX equivalent of that HTML tag.

A real example of how this would be helpful

I'm building out this page in MDX:

image

The legend, in particular, looks like this:

image

I've built the legend in the MDX file with:

<ul className="...">
    <li className="">...</li>
    <li className="">...</li>
    <li className="">...</li>
    <li className="">...</li>
</ul>

I want the <ul> and <li>s to look different than a <ul> created with:

* This `<ul>`
* should render with
* three bullet points

When I use custom components in MDXProvider, there seems to be no way to distinguish between a ul created with JSX and a ul created with MDX:

<MDXProvider
    components={{
        ul: (props) => {
           // How can I tell if this started as JSX or MDX? If it's JSX, I'd like to
           // return early and just do `<ul {...props} />`.
        },
    }}
>
    {children}
</MDXProvider>

Your environment

Steps to reproduce

N/A

(Happy to make one if the issue needs clarification.)

Expected behaviour

N/A

Actual behaviour

N/A


I hope this question made sense. I'm having trouble phrasing it but am happy to elaborate more if it's confusing. Thanks for all the work you all have done! 🙇

@johno
Copy link
Member

johno commented Apr 30, 2019

It does make sense! And thanks for opening up an issue.

Currently, there isn't a built in way to detect a component that's coming from Markdown syntax or JSX.

This was a decision based on a lot of conversations where we ultimately decided it was more confusing to folks when JSX elements (like an h1 or p) were rendered differently than the Markdown equivalent. Especially when you consider plugins that inject HTML. This unfortunately comes with the downside of not being able to detect whether the component being rendered is coming from MD syntax or JSX.

In general, it seems the "happy path" would be to create a new component for your legend (which can now become a global shortcode via the MDXProvider).

If this approach is problematic for you we could also consider passing some type of magic prop in the pragma so users can detect the difference between JSX and Markdown components.

@danoc
Copy link
Author

danoc commented Apr 30, 2019

Thanks for explaining the context!

This was a decision based on a lot of conversations where we ultimately decided it was more confusing to folks when JSX elements (like an h1 or p) were rendered differently than the Markdown equivalent. Especially when you consider plugins that inject HTML. This unfortunately comes with the downside of not being able to detect whether the component being rendered is coming from MD syntax or JSX.

Makes sense. Previously I had worked around this by doing imports like this in my MDX:

import { InlineCode, H2, P } from 'components/mdx';

Personally I thought the workaround was great, but I can see how it'd be a pain-point for folks trying to mix MDX and JSX.

If this approach is problematic for you we could also consider passing some type of magic prop in the pragma so users can detect the difference between JSX and Markdown components.

That would work—although I can see other developers on my team finding it confusing.

Would it be difficult to make that information officially supported in MDXProvider? If it's not possible, I agree that the happy path would be to create a new component for the legend or anything else that needs to be JSX.

@johno
Copy link
Member

johno commented May 1, 2019

Just to clarify, I'm wondering what you mean by:

other developers on my team finding it confusing

Does that mean using a component in place of the ul/li?:

import Legend from './place'

# A heading

Other stuff

<Legend.ul className="...">
  <Legend.li className="">...</Legend.li>
  <Legend.li className="">...</Legend.li>
  <Legend.li className="">...</Legend.li>
  <Legend.li className="">...</Legend.li>
</Legend.ul>

Would it be difficult to make that information officially supported in MDXProvider?

It might be a little tricky since we don't know where the element comes from (MD syntax or JSX) by the time it reaches the custom pragma which handles the React element creation so we'd have to add some sort of property which would be present in the compiled JSX.

You could also use some type of wrapping component that clones/modifies its children:

import Embed from './place'

# A heading

Other stuff

<Embed>
  <ul className="...">
    <li className="">...</li>
    <li className="">...</li>
    <li className="">...</li>
    <li className="">...</li>
  </ul>
</Embed>

@johno
Copy link
Member

johno commented May 1, 2019

One other random idea, for your use case can you key off the className?

@johno johno added the 🙉 open/needs-info This needs some more info label May 14, 2019
@danoc
Copy link
Author

danoc commented Jun 12, 2019

Sorry for the delay here. Started working on other stuff and haven't had much time to think about this.


One other random idea, for your use case can you key off the className?

Yeah, this would work fairly well. The Code nodes would need special treatment since they sometimes have a className such as language-jsx if the language was specified alongside the triple backticks.

Just to clarify, I'm wondering what you mean by:

other developers on my team finding it confusing

Does that mean using a component in place of the ul/li?:

I meant that they'd find it confusing that the the HTML they wrote is getting transformed. The workarounds are fairly simple though.


Anyways, feel free to close the issue. I personally prefer the old behavior, but it's not a huge deal. Thanks for the help and for all the work you've done on MDX!

@johno johno added 🦋 type/enhancement This is great to have 💬 type/discussion This is a request for comments 📚 area/docs This affects documentation and removed 🙉 open/needs-info This needs some more info labels Jul 31, 2019
@johno
Copy link
Member

johno commented Jul 31, 2019

Yeah, this is something that we need to be able to solve so I'm going to leave this open while I ponder the best solution or at least better document things.

It's definitely been a source of confusion where some folks expect HTML to maintain the same styles/component rendering as their Markdown equivalent while others don't. 🤷‍♂

@EmaSuriano
Copy link

Any updates on this issue?

@johno
Copy link
Member

johno commented May 21, 2020

We’ll likely implement some sort of API to allow opting out of the MDXProvider rendering (#821) though that might not fully address this issue.

The above strategy (API still TBD) will ship with v2.

@wooorm
Copy link
Member

wooorm commented Nov 13, 2020

As I believe #821 solves this, and is closed and in MDX@2, I’ll close this here too

@wooorm wooorm closed this as completed Nov 13, 2020
@wooorm wooorm added 💎 v2 Issues related to v2 💪 phase/solved Post is done and removed 💬 type/discussion This is a request for comments 📚 area/docs This affects documentation labels Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 phase/solved Post is done 🦋 type/enhancement This is great to have 💎 v2 Issues related to v2
Development

No branches or pull requests

4 participants