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

🏗 Improve JSX block parsing #195

Closed
johno opened this issue Jul 23, 2018 · 19 comments
Closed

🏗 Improve JSX block parsing #195

johno opened this issue Jul 23, 2018 · 19 comments
Assignees
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧑 semver/major This is a change 🦋 type/enhancement This is great to have

Comments

@johno
Copy link
Member

johno commented Jul 23, 2018

A continuation of #176 that will handle JSX blocks using a custom block parser that extends remark. This will ensure that JSX blocks that contain empty new lines are parsed as a single, cohesive block.

@geddski
Copy link

geddski commented Aug 21, 2018

looking forward to this! It will let me have spaces in my code examples

@wooorm
Copy link
Member

wooorm commented Aug 26, 2018

One thing I’m wondering is whether it would make sense to, as I believe currently JS is embedded in markdown, to swap it around and do markdown in JS.

But I’m guessing what makes the most sense is to remove the html tokeniser and add a custom JSX tokeniser instead (edit: ah, that’s what you’re proposing 🙃)? Something that swallows indented blank lines for example? Or that tracks open and closing tags to find out if the first tag is closed already?

@kizu
Copy link

kizu commented Mar 12, 2019

Not sure if this is the issue to mention this or I should create a new one, but it is surely a thing that would be nice to fix in the JSX block parsing: tagged template literals. Right now those throw this error:

SyntaxError: Unexpected token var

Tagged template literals are often used with styled-components, for example, so it would be really nice to fix the support for those.

@johno
Copy link
Member Author

johno commented Mar 12, 2019

Yeah, absolutely @kizu!

Could you paste an example MDX file causing the issue and let us know what versions of MDX libraries you're running? Theoretically tagged template literals were fixed in a recent release, but sounds like there may have been a regression.

@kizu
Copy link

kizu commented Mar 12, 2019

Hmm, I couldn't reproduce it quicky at gatsby on codesandbox, but I encountered this issue both at Docz, by trying to create a component inside like this:

import { Playground } from 'docz'
import styled from 'styled-components'

<Playground>
  {() => {
    const Foo = styled.div`
      background: pink;
    `
    return <Foo>Hello</Foo>
  }}
</Playground>

This gives this error, while doing this (using ([]) instead of tagged temlpate) works:

<Playground>
  {() => {
    const Foo = styled.div([`
      background: pink;
    `])
    return <Foo>Hello, world</Foo>
  }}
</Playground>

Trying to use a tagged temlpate at mdxjs.com live examples also gives the same error:

image

(doing there regular concat([]) works as well, of course).

@johno
Copy link
Member Author

johno commented Mar 12, 2019

Perfect, thank you @kizu. We'll get on addressing this issue.

@jxnblk
Copy link
Contributor

jxnblk commented Apr 10, 2019

FWIW I'm not sure I have all the context for this issue, but for the most part the way MDX currently behaves seems similar to how markdown works with HTML: if there are newlines around the text in an HTML block, then that text is parsed as markdown, and without the newlines, then it's raw text.

For example (it should work in this GitHub comment):

<div>
# raw text
</div>

<div>

# Heading

</div>
# raw text

Heading

Here are related examples from the CommonMark spec:

There might still be issues with nested elements, but I thought I'd chime in here if it's helpful

@gchallen
Copy link

Any updates on this? Template literals that include empty lines are still not working AFAICT, and this makes it impossible to pass code strings to components through props:

<Playground reference={`
public class Whatever() {
    // The blank line below will cause parsing to fail.

}
`} />

@philj0st
Copy link

philj0st commented Jun 28, 2019

using Reactstrap is there a way to let markdown like this be parsed correctly?

<Row>
  <Col>
  - some md
  - asd
  - asd
  </Col>
  <Col>
  **some md**
  </Col>
</Row>

Am I doing something wrong?
this leads to everyting on one line for me:

- some md - asd - asd                                           **some md**

@millette
Copy link
Contributor

@philj0st see #195 (comment)

Basically, pad your custom tags with empty lines. It's just a hack though.

@philj0st
Copy link

philj0st commented Jul 1, 2019

thanks @millette this helps a lot.
But then there's no way to make the following work I assume. or am I missing something?

<Col>

- osm
- <Icon name="hero" /> **This will be plain text, not MD** <----
- **some md**

</Col>

@millette
Copy link
Contributor

millette commented Jul 1, 2019

@philj0st You'd have to try it with empty lines around <Icon /> I'm not sure how that will affect the bullet-list.

@Danielku15
Copy link

I just started to use Docusaurus for my docs which use MDX. And it is quite a big pain in formatting the docs on code level without the support of empty lines. I think it would already help a lot if we could start/end JSX blocks explicitly with some syntactic sugar. In many cases I would not mind wrapping my JSX specific code to a special block like:

Currently not supported:

---
title: MyPage
---

export function buildPropertyUrl(property) {
	let url = '';

	if (property.prop('todo', false)) {
		url = "#todo";
	} else if (url = child.prop('link')) {
		url = useBaseUrl(url);
	} else {
		url = property.getUrl();
	}

	return url;
}

# Hello world
{buildPropertyUrl(someobj)}

Alternative with (some) wrapper:

---
title: MyPage
---

<Jsx>
export function buildPropertyUrl(property) {
	let url = '';

	if (property.prop('todo', false)) {
		url = "#todo";
	} else if (url = child.prop('link')) {
		url = useBaseUrl(url);
	} else {
		url = property.getUrl();
	}

	return url;
}
</Jsx>

# Hello world
{buildPropertyUrl(someobj)}

Of course some discussion on the actual syntax would be needed but I guess it would be the easiest solution until the auto detection of blocks is improved.

@johno
Copy link
Member Author

johno commented Apr 14, 2020

I'm going to document here (as a single comment) the primary parse issues we're seeing so that we (@wooorm and I) can begin addressing them. If I'm missing common cases here, please feel free to chime in and I'll add them!

Template string with empty line

# Hello, world!

<Foo
  stuff={`
Here is a template literal

with an empty line
  `}
/>

Export with empty line

export const Foo = () => {
  const myValue = fetchSomeStuff()

  return <h1>{myValue}</h1>
}

# Hello, world!

<Foo />

Fragment with JSX expression

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

Weeeeee!

Render props with empty line

# Hello, world!

<Foo>{
  const num = 1+1
  
  return <h2>{num}</h2>
}</Foo>

@AleksandrHovhannisyan
Copy link

@johno Are there any workarounds in the meantime? I'm running into the template literal issue.

@ChristianMurphy
Copy link
Member

@AleksandrHovhannisyan according to #1039 (linked right above you comment) this should be fixed in version 2.
You can find out more information about what changes in version 2 and how to try it out in #1041

@wooorm
Copy link
Member

wooorm commented Dec 18, 2020

Hi 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 and removed 💎 v1 Issues related to v1 labels Dec 18, 2020
@wooorm wooorm unpinned this issue Dec 18, 2020
@Stvad

This comment has been minimized.

@ChristianMurphy
Copy link
Member

@Stvad https://github.com/mdx-js/mdx/discussions is a good place to ask questions

@wooorm wooorm added the 💪 phase/solved Post is done label Dec 24, 2021
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/enhancement This is great to have
Development

No branches or pull requests