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

RFC: Interleaving Markdown in JSX #628

Closed
johno opened this issue Jul 11, 2019 · 58 comments · Fixed by #1039
Closed

RFC: Interleaving Markdown in JSX #628

johno opened this issue Jul 11, 2019 · 58 comments · Fixed by #1039
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧑 semver/major This is a change 💬 type/discussion This is a request for comments 🦋 type/enhancement This is great to have 💎 v2 Issues related to v2
Milestone

Comments

@johno
Copy link
Member

johno commented Jul 11, 2019

Summary

Interleaving Markdown and JSX in MDX has been asked for by a lot of folks and is something we should officially support. It's been on the roadmap, but we've decided to add it as a minor version after v1. Now is that time 😸

It's a common need to wrap a block of content to achieve things like image + captions, notifs, callouts, and custom background/text colors for given sections.

Basic (proposed) example

Inline JSX blocks are automatically interleaved and block-level JSX requires an blank lines to open back up Markdown parsing:

# Hello, <span>*world*</span>, from <>{props.name}</>!

<Note>

This *is* **markdown**!

</Note>
Output JSX
<MDXLayout
  {...layoutProps}
  {...props}
  components={components}
  mdxType="MDXLayout"
>
  <h1>
    {`Hello, `}
    <span>
      <em parentName="h1">{`world`}</em>
    </span>
    {`, from `}
    <>{props.name}</>
    {`!`}
  </h1>
  <Note mdxType="Note">
    <p>
      {`This `}
      <em parentName="p">{`is`}</em>
      {` `}
      <strong parentName="p">{`markdown`}</strong>
      {`!`}
    </p>
  </Note>
</MDXLayout>

Motivation

This approach is currently being used in the wild and in production. As @jxnblk mentioned previously this approach comes with the benefit that it's in the Commonmark Spec.

Detailed design

The majority of parsing is already implemented. However, there are a few things we need to be able to address.

Handling embedded expressions {props.foo}

There are two additional scenarios where we need to be able to handle embedded expressions, which all essentially boil down to inline JSX.

# Hello, from <>{props.name}</>

<Note>

Here's another expression: <>{props.thing}</>

</Note>

This can get a bit tricky because technically the following should also be handled:

# Hello, <>from {props.name}</>

Handling more complex JSX blocks

When there is indented JSX across two lines the parser currently misses it. Something that doesn't currently work, but should:

# Hello, world!

<div>
  <div>

# I should be an h1 inside two divs

  </div>
<div>

Drawbacks

This will likely make the last JSX block parsing issue more difficult. The biggest remaining block issue is where we have empty lines in the JSX. This causes parsing breakages for render props and template strings.

Render props

# Hello, world

<SomeComponent>
  {prop => {
    const newValue = doStuff(prop)

    return <h1>{newValue}</h1>
  }}
</SomeComponent>

Template strings

# Hello, world!

<SomeComponent
  someProp={`
    Here's a template string

    with empty lines
  `}
/>

Alternatives

The other main alternative we've considered is an MDX component.

# Hello, <span><MDX>*world*</MDX></span>!

<Note>
  <MDX>
    This *is* **markdown**!
  </MDX>
</Note>

An additional alternative from PHP Markdown Extra is using a markdown attribute:

# Hello, <span markdown="1">*world*</span>!

<Note markdown="1">
  This *is* **markdown**!
</Note>

This comes with the benefit of being more explicit, but it is confusing for folks coming from traditional Markdown as they slowly adopt JSX. It also feels syntactically noisy.

I've toyed with different syntax in place of MDX like aliasing it to something terser: <_>. However, this isn't as approachable.

Adoption strategy

Considering this already mostly works and is already being used (just not officially supported) the adoption strategy should be straightforward. We'd need to fix some parsing edge cases mentioned above and add documentation.

Related issues


cc/ @wooorm @jxnblk @ChristopherBiscardi @timneutkens @ChristianMurphy @silvenon

@johno johno added 💬 type/discussion This is a request for comments rfc labels Jul 11, 2019
@ChristopherBiscardi
Copy link
Member

It's probably worth mentioning at the beginning what the output of span/world should be as fragments are mentioned later for interpolation but not for interleaving

# Hello, <span>*world*</span>!

<Note>

This *is* **markdown**!

</Note>

@johno johno pinned this issue Jul 11, 2019
@silvenon
Copy link
Contributor

silvenon commented Jul 11, 2019

What should happen in this case?

# Hello, <>from {props.name}</>

It feels to me that if this worked, this should work as well:

# Hello from {props.name}

These things are hard 😓

@johno
Copy link
Member Author

johno commented Jul 11, 2019

What should happen in this case?

# Hello, <>from {props.name}</>

Yeahhh, good point. I wonder if we should only allow embedded expressions to be the only "children" in a JSX block (whether inline or block - <>{props.name}</>)? That would make the parsing easier for the render props case, too. 🤔

@silvenon
Copy link
Contributor

Yeah, I think that would be a win-win because it wouldn't be syntactically misleading.

@rchrdnsh
Copy link

would my use case in this issue:

#474

...be solved by this kind of feature? I realized that I'm really wanting to wrap different lists in different styles, but still write the lists in markdown, but then include jsx in the markdown, like so:

<ListStyleOne>

  -  Thing
      -  Subthing and this word is <Red>Red</Red> right here.
      -  Another subthing with a <Blue>colorful</Blue> word again.

</ListStyleOne>

it seems like this is the case initially, but I just want to be sure that I'm understanding this proposal correctly XD

@johno
Copy link
Member Author

johno commented Jul 12, 2019

would my use case in this issue:

#474

...be solved by this kind of feature? I realized that I'm really wanting to wrap different lists in different styles, but still write the lists in markdown, but then include jsx in the markdown

It would 😄

@rchrdnsh

This comment has been minimized.

@wooorm
Copy link
Member

wooorm commented Jul 18, 2019

This looks good! I think it makes sense for users. But there’s still some parser problems. Here goes a few more questions and random thoughts.

  • block-level JSX requires an empty newline

    To make sure we’re on the same page: I think you mean a blank line, namely two newlines (\n\n), optionally with white space (as defined by CM: https://spec.commonmark.org/0.29/#blank-lines). A newline is a carriage return and/or line feed. And line feed / carriage return are the unix / windows characters!

  • The first example:

    # Hello, <span>*world*</span>, from <>{props.name</>!

    Turns into (zooming into world):

    <span>
      <em parentName="h1">{`world`}</em>
    </span>

    …is parentName indeed h1? 🤔
    P.S. props.name misses a closing curly brace

  • I don’t think # Hello from {props.name} should be supported (@silvenon). Users are already learning that < and > need to be there to be in JSX. However, implementing it everywhere is easier than only inside inline JSX.

  • I think <>from {props.name}</> shouldn’t be too tricky. We start out with Markdown. If </> or other JSX is found, either inline or block level JSX is entered. We add a new inline tokenizer to remark that only works “in” JSX. This means we need to match open and closing tags for inline JSX though, but that’s doable. As {} are not used in “normal” Markdown, we could just as well support it everywhere though.

Drawbacks

Interesting insights: MDX in JSX blocks in MDX is typically not indented, and needs two blank lines. I think that’s too fragile to depend on though. 🤔

@rsexton404
Copy link

Would it make the parsing more explicit if the JSX block included a markdown prop/attribute? There's prior art here from PHP Markdown Extra that is worth reviewing.

@johno
Copy link
Member Author

johno commented Jul 31, 2019

Thanks for the thoughts @wooorm! I've addressed a few things in edits to the original comment.

…is parentName indeed h1?

Yeah I think this is something we'll need to also address since, you're right, it isn't 😆.

Would it make the parsing more explicit if the JSX block included a markdown prop/attribute? There's prior art here from PHP Markdown Extra that is worth reviewing.

Thanks for pointing this out @rsexton404! I've added it as an alternative in the comment. It'd definitely make parsing easier due to it's explicitness, but it's also a bit noisy. I'll need to think on that one!

@wooorm
Copy link
Member

wooorm commented Jul 31, 2019

Re markdown attribute, could be mdx as well:

# Hello, <span markdown="1">*world*</span>!

<Note mdx={true}>
  This *is* **markdown**!
</Note>

@sdegutis

This comment has been minimized.

@sdegutis

This comment has been minimized.

@johno

This comment has been minimized.

@sdegutis

This comment has been minimized.

@wooorm

This comment has been minimized.

@sdegutis

This comment has been minimized.

@sdegutis

This comment has been minimized.

@sdegutis

This comment has been minimized.

@ChristianMurphy

This comment has been minimized.

@sdegutis

This comment has been minimized.

@sdegutis

This comment has been minimized.

@JamesMessinger
Copy link

JamesMessinger commented Aug 22, 2020

Thanks for the fast reply @wooorm. The first example you showed would work for a single JSX slot, but my component actually allows an arbitrary number of slots:

<ProductCatalog products={[
    {
        productName: "Product A",
        description: <div>This *is* **markdown**</div>
    },
    {
        productName: "Product B",
        description: <div>This *is* **markdown**</div>
    },
    {
        productName: "Product C",
        description: <div>This *is* **markdown**</div>
    },
]}/>

The only workaround I can think of that still allows for Markdown content is to make each slot a child component:

<ProductCatalog>
  <Product name="Product A">
    <ProductDescription>
      <div>This *is* **markdown**</div>
    </ProductDescription>
  </Product>

  <Product name="Product B">
    <ProductDescription>
      <div>This *is* **markdown**</div>
    </ProductDescription>
  </Product>

  <Product name="Product C">
    <ProductDescription>
      <div>This *is* **markdown**</div>
    </ProductDescription>
  </Product>
]}/>

This works, and supports Markdown content (in MDX v2). 👍 It's just tedious and feels a lot more like writing XML than Markdown 😟

@wooorm
Copy link
Member

wooorm commented Aug 22, 2020

Great to hear that that works! Is this good enough for you? Or do you feel very strongly that more markdown should also be available in props?

I think that that would be technically very hard. Although, maybe the MDX component you showed before could be implemented in userland, if that also works?

@JamesMessinger
Copy link

I would expect any JSX component in an MDX file to support Markdown content, regardless of where the component occurs. Examples include:

export const MyComponent = <div>This *is* **markdown**</div>

export const ArrayOfComponents = [
    <div>This *is* **markdown**</div>,
    <div>This *is* **markdown**</div>,
    <div>This *is* **markdown**</div>
];

<SomeComponent someProp={<div>This *is* **markdown**</div>} />

<ProductCatalog products={[
    {
        productName: "Product A",
        description: <div>This *is* **markdown**</div>
    },
    {
        productName: "Product B",
        description: <div>This *is* **markdown**</div>
    },
    {
        productName: "Product C",
        description: <div>This *is* **markdown**</div>
    },
]}/>

Let me emphasize that while I think it would be totally awesome to support all of the above examples, I realize this means you'd basically have to write a JavaScript parser. 😰 So, no, I don't feel strongly that this is a must-have feature. MDX is already friggin' awesome, crazy powerful, and incredibly useful.

@wooorm
Copy link
Member

wooorm commented Aug 22, 2020

Your last observation is indeed correct, to rephrase: we parse markdown and recognise the JSX inside it. Your expectation is the inverse: parse JavaScript and recognise markdown inside it. It sounds interesting, but to me it doesn’t align well with this project, or at least my ideas of how content should work, maybe others will have different opinions though!

@wooorm
Copy link
Member

wooorm commented Aug 22, 2020

And thanks, glad to hear MDX is generally working well for you! Also: do let us know more about your experience w/ mdx@next!

@wooorm
Copy link
Member

wooorm commented Dec 18, 2020

Hi y’all! I’m going to close this as it landed on the main, the (now default) branch we’re using to gear up to the MDX@2 release.

The reason is so that it’s more clear which issues still need to be solved already and don’t require further work.

Expect another next release soonish, which might include it, or maybe it’s already released as a beta!

For more info, see #1041.

@wooorm wooorm closed this as completed Dec 18, 2020
@wooorm wooorm added ⛵️ status/released 🗄 area/interface This affects the public interface 🦋 type/enhancement This is great to have 🧑 semver/major This is a change labels Dec 18, 2020
@wooorm wooorm added the 💪 phase/solved Post is done label Dec 24, 2021
@CEbbinghaus
Copy link

Wonderful feature, I wanted to add a comment on my particular use case which is the same as JamesMessinger.

I would expect any JSX component in an MDX file to support Markdown content, regardless of where the component occurs. Examples include:

export const MyComponent = <div>This *is* **markdown**</div>

export const ArrayOfComponents = [
    <div>This *is* **markdown**</div>,
    <div>This *is* **markdown**</div>,
    <div>This *is* **markdown**</div>
];

<SomeComponent someProp={<div>This *is* **markdown**</div>} />

<ProductCatalog products={[
    {
        productName: "Product A",
        description: <div>This *is* **markdown**</div>
    },
    {
        productName: "Product B",
        description: <div>This *is* **markdown**</div>
    },
    {
        productName: "Product C",
        description: <div>This *is* **markdown**</div>
    },
]}/>

While it is pretty unrealistic to expect mdx to support markdown within JSX as this would be incredibly difficult, It does add an additional layer of consideration. As now the behavior is different dependent on whether a component is at the root layer / a direct child thereof, or whether a component is part of a property (e.g jsx)

Consider the following two examples:

<SomeComponent>
<>**Some *Markdown***</>
</SomeComponent>

<SomeComponent Content={<>**Some *Markdown***</>}/>

The behavior differs between them which is something any Developer used to JSX/TSX is not going to expect. In Most/All Modern frameworks there is almost complete consistency between the HTML & JSX Approach of defining content which does not translate well to MDX.

This is something that the alternate proposal does not have a problem with. As the content is clearly marked as mdx and as such can be parsed even within JSX code.

<SomeComponent Content={<MDX>**Some *Markdown***</MDX>}/>

I feel it would be worthwhile to pursue as not a replacement of of current system but rather an addition to. Giving a pretty consistent feel for any top level content while allowing devs to still utilize mdx within JSX.

@ChristianMurphy
Copy link
Member

Thanks for sharing @CEbbinghaus!
I'd second @wooorm's response to the first time it was brought up.

It sounds interesting, but to me it doesn’t align well with this project, or at least my ideas of how content should work

There are other ways to support this.
In particular template literal something like

<SomeComponent Content={mdx`**Some *Markdown**`}/>

⚠️ I'd avoid using a JSX tag like

<SomeComponent Content={<MDX>**Some *Markdown**</MDX>}/>

as JSX handles spacing and newlines in a different and conflicting way from markdown ⚠️

@CEbbinghaus
Copy link

CEbbinghaus commented Mar 12, 2024

I saw your comment recommending a template literal to pass into components1. However there is no existing template literal that will do this & the ones you can utilize require mdx at runtime as it won't be evaluated at compile time. I have nothing against the template literal approach although whichever approach taken it would be good to have official support for it and ensure it is consistent (e.g it builds at compile time). Additionally if template literals are the preferred method it would be good to integrate that into the LSP to allow for the same autocomplete behavior as when editing anything else.

The Dev Experience is something I put a lot of value on and I would like nothing more than to make MDX feel as consistent and smooth as it can. Please do correct me if I am wrong about there being no built in template literal. I am happy to contribute 👍🏻

Footnotes

  1. https://github.com/orgs/mdx-js/discussions/1954#discussioncomment-2262015

@remcohaszing
Copy link
Member

You could write a recma plugin that finds mdx template literals and compiles the content at build time. I don’t think this should be a part of MDX itself, but we’ll gladly add it to our list of known third party plugins. Plugin support in the language server is still under investigation.

@wooorm
Copy link
Member

wooorm commented Mar 12, 2024

@CEbbinghaus This is an older issue, about adding interleaving in general. I’d feel more comfortable discussing more and different interleaving methods in a new discussion, so as to keep issues focussed.

Your idea is fundamentally very complex due to several things. One reason is that MDX also includes ESM and expressions. In those, entire functions and components can be defined, dynamically.

export function SomeComponent() {
  return <>Is this markdown?</>
}

{
  (function () {
    return <>Is this markdown?</>
  })()
}

The other part of why “interleaving in JS” is tough, is that it is unclear what “markdown content type” is expected.

<X x={<>- Is this a list?</>} y={<>Is this a paragraph?</>} z={<>Is this *emphasis*, what about the outer paragraph?</>} />

Finally, JSX text also has certain handling that is present in Babel, Acorn, TypeScript, etc, which trims whitespace, also around line endings. Whereas whitespace is important to markdown. You’d have to either break from JS(X) or from markdown.

Indeed, you can do things with plugins.
Plugins mean that we don’t have to decide something here that will apply to everyone.
You can make a plugin to do what you want and share it.
If then we find that basically everyone wants your new idea, we can always add it here.

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 💪 phase/solved Post is done 🧑 semver/major This is a change 💬 type/discussion This is a request for comments 🦋 type/enhancement This is great to have 💎 v2 Issues related to v2
Development

Successfully merging a pull request may close this issue.