Bug: working with render props

[pedronauck]

Isn't possible to reproduce render props component like that:

import { Foo } from './Foo'

# Hello Foo

<Foo>
  {() => {
    const bar = 'bar'

    return (
      <Bar bar={bar} />
    )
  }}
</Foo>

I noticed that in the MDAST => MDXAST process you just turn HTML into JSX nodes, but in this case the line breaks between const and return is a trade off, because the parse is a Paragraph instead of the same scope of HTML.

So, I was trying to figure out how fix this and I think that what can be a possible solution is create something like a merge between nodes when the HTML node that has the opening tag is invalid (without enclosing tag). I don't know if this can generate some trade offs or is a solution that fit in all cases. So, I didn't think yet how this can affect performance, but was the only thing that came in mind in that case 😞

[timneutkens]

What's the use case for doing render props inside mdx 🤔 Note that mdx only supports a few things from javascript, being import export and jsx tags

[pedronauck]

There's so many times that you can use render props as a container to encapsulate some logic then doing something with it props:

<Container>
  {(data}) => {
    const modifiedData = modify(data)

    return (
      <MyComponent prop={modifiedData} />
    )
  }
</Container>

Ok, in this case you could pass modify(data) directly as a prop for <MyComponent /> but what I'm trying to show here is that this should work with the expected behavior without gotchas!

[renatorib]

A common use case: use mdx to documentation your components. Components can be render props like any other pattern.

For example: using https://github.com/renatorib/react-powerplug

# <MyDumbCheckbox> Component

<Toggle>
  {({ on, toggle }) => (
    <MyDumbCheckbox selected={on} onClick={toggle} />
  )}
</Toggle>

Or without powerplug, only raw component behavior

# <MySmartCollapse> Component

<MySmartCollapse>
  {({ opened, toggle }) => (
    <>
      <div onClick={toggle}>Toggle collapse content</div>
      {opened && <div>Hidden content</div>}
    </>
  )}
</MySmartCollapse>

[timneutkens]

So the only reason Pedro’s example doesn’t work is the empty line break between const and return, remark Will parse that as 2 paragraph nodes

[renatorib]

Ah, got it!
Anyway this does not invalidate the usecase for "doing render props inside mdx".

[timneutkens]

So what I meant to say is that it shouldn’t be a special case, we 1 on 1 copy the jsx nodes into the final mdx document, so anything inside jsx tags won’t be touched by us

[timneutkens]

Sorry for the close/open, writing on my phone and hitting the wrong button 😅

[renatorib]

Got it

[pedronauck]

@timneutkens I did a pull request proposing a fix for this situation, if you can take a look it would be cool ✌️

[fzaninotto]

I'm also +1 with render props ; It's becoming such a common pattern that many components can't be used without it.

[timsuchanek]

A workaround that I'm using:

import { Foo } from './Foo'

# Hello Foo

<Foo chilren={() => {
   const bar = 'bar'

   return (
    <Bar bar={bar} />
   )
}} />

However, now I would like to embed markdown in the return statement:

import { Foo } from './Foo'

# Hello Foo

<Foo chilren={() => {
   const bar = 'bar'

   return (
    <div>
      
     ## h2

      <Bar bar={bar} />
    </div>
   )
}} />

Which unfortunately doesn't interpolate the markdown. Any ideas on this?

[silvenon]

As a user, I don't think MDX should be able to do this. It reduces readability and seems like a maintenance nightmare.

The way I see it, MDX is supposed to be Markdown with occasional embedded JSX. Your example features Markdown with embedded JSX with embedded JavaScript returning JSX with embedded Markdown.

I think the burden of resolving that kind of complexity should fall onto the user. Depending on your use case, you could split these into multiple MDX files, do some voodoo with the components prop etc.

If you have a real-world example where MDX falls short and you would like to share it, please do!

[timsuchanek]

I agree that it shouldn't necessarily support arbitrary nesting of jsx/markdown.

My use-case is a tooltip component:

export const content = `
  ## This is tooltip content
  *blub*
`

<Tooltip placement="top" renderTooltip={content}>
  {({ref}) => (
    <span ref={ref}>
      This is a link
    </span>
  )}
</Tooltip>

This is my solution now, then I'm calling react-remark in the Tooltip component to parse the markdown.

[silvenon]

react-remark sounds like an acceptable workaround for this presumably rare situation. Also, you shouldn't have to export content only to use its value. Doesn't using the value directly work?

<Tooltip
  placement="top"
  renderTooltip={`
    ## This is tooltip content
    *blub*
  `}
>
  {({ref}) => (
    <span ref={ref}>
      This is a link
    </span>
  )}
</Tooltip>

Btw, the content of tooltips usually isn't long, if that's the case here, perhaps Markdown is an overkill?

[erquhart]

I agree w/ @silvenon's original comment - I didn't expect MDX to essentially become JavaScript-in-Markdown. This has major implications for the portability of MDX documents. Curious if this was always the plan (versus supporting only certain value types as props in MDX components)?

[timneutkens]

Just to give some context to @erquhart's comment/question:

For the initial MDX implementation I kept in mind a very strict superset of Javascript (or not even JS, as JSX is not JS), the only thing MDX parses are import export and jsx jsx is handled by unified, we convert the html nodes to jsx nodes for consistency. import/export are parsed in the simplest way possible, if the ast node starts with import/export it's an import/export, if not it's normal text.

I've always thought that writing render props / other JS in MDX documents doesn't make sense as it introduces unnecessary complexity and there are other ways to handle it, for example as you can use JSX you could move that code into a component and make that component render everything you need.

It's also a good idea to keep in mind that soon render props will in most cases be replaced by Hooks.

The only case that is interesting to explore deeper is the possibility to write JSX inside of markdown syntax like **<Something />** which is currently not possible. I know @johno has been playing around with this idea for a while though.

[huchenme]

it is sad that render props is not working in mdx, we are using docz for documentation, although docz added its own render prop plugins, prettier still not able to format file correctly because of this

[johno]

Render props do work, unfortunately it's a current limitation in the parser that doesn't handle empty lines in JSX blocks. I'm going to close this for now since we can track progress in #195.