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

remark-mdx: empty line before closing tag issue #767

Closed
atanasster opened this issue Sep 7, 2019 · 15 comments · Fixed by #1039
Closed

remark-mdx: empty line before closing tag issue #767

atanasster opened this issue Sep 7, 2019 · 15 comments · Fixed by #1039
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 👶 semver/patch This is a backwards-compatible fix 🐛 type/bug This is a problem

Comments

@atanasster
Copy link

Subject of the issue

The issue causes parsing failure in a compiler plugin that parses the child.value strings for post processing.

Reproduce

The following:

<button>
  <div>hello world</div>
  //<- there is one empty space here at the start of the line
</button>

will parse as

  "children": [
    {
      "type": "jsx",
      "value": "<button>\n  <div>hello world</div>\n\n</button>\n\n",
...
    }
  ],

However, removing the one space on the empty line will parse as 3 separate elements:

<button>
  <div>hello world</div>
//<- there is no empty space here
</button>
 "children": [
    {
      "type": "jsx",
      "value": "<button>\n  <div>hello world</div>",
    },
    {
      "type": "text",
      "value": "\n"
    },
    {
      "type": "jsx",
      "value": "</button>",
    }
  ],

Potential solution

The issue seems to be here:

if (sequence[1].test(line)) {

Since an empty string will always pass the test, the closing tag is missed if there is an empty line before it.

Any reasons not to change the rule to

   ...
      if (line.length && sequence[1].test(line)) {
        index = next
        break
      }
...
@atanasster atanasster added 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Sep 7, 2019
@atanasster
Copy link
Author

@johno - did you have a chance to check this issue?

@Zunaib
Copy link

Zunaib commented Dec 27, 2019

This still occurs, any workaround on the way ?

@threehams
Copy link

threehams commented Jan 22, 2020

Current workaround is to avoid any empty lines within JSX blocks as they may or may not cause a syntax error. Prettier + many editors strip out trailing whitespace, so I don't think you're going to want to rely on it.

@seanhealy
Copy link

This is still being problematic for us. Removing empty lines isn't great as it leaves the example code harder to read. Has anyone found a reliable way of preventing Prettier from stripping trailing whitespace within mdx Playground blocks?

@FG-avi
Copy link

FG-avi commented Apr 17, 2020

I was also having this issue. The error message I got was:

"SyntaxError: unknown: Adjacent JSX elements must be wrapped in an enclosing tag. Did you want a JSX fragment <>".

I am putting the error message in here since searching for the error did not lead me to this issue. I just happened to read a comment that had a link to here.

@Iulia-Soimaru
Copy link

Iulia-Soimaru commented Apr 23, 2020

I am getting similar errors as well, my Story:

<Story name="input">
    {() => {
        const [value, setValue] = useState('');

        return (
            <InputComponent
                placeholder="0"
                value={value}
                onChange={setValue}
            />
        )
    }}
</Story>

error:

Module build failed (from ./node_modules/@mdx-js/loader/index.js):
SyntaxError: unknown: Adjacent JSX elements must be wrapped in an enclosing tag. Did you want a JSX fragment <>...</>? (64:0)

  62 | </Story>
  63 | `}</code></pre>
> 64 | </Preview>
     | ^
  65 | <h1 {...{"id":"api"}}>{`API`}</h1>
  66 | <Props of={InputComponent} />
  67 |     </MDXLayout>

and when I remove space before return:

<Story name="input">
    {() => {
        const [value, setValue] = useState('');
        return (
            <InputComponent
                placeholder="0"
                value={value}
                onChange={setValue}
            />
        )
    }}
</Story>

getting this error instead:

Module build failed (from ./node_modules/@mdx-js/loader/index.js):
SyntaxError: Unexpected token (49:12)
    at Object._raise (/Users/iuliasoimaru/Documents/www/platform/node_modules/@babel/parser/lib/index.js:742:17)
    at Object.raiseWithData (/Users/iuliasoimaru/Documents/www/platform/node_modules/@babel/parser/lib/index.js:735:17)
    at Object.raise (/Users/iuliasoimaru/Documents/www/platform/node_modules/@babel/parser/lib/index.js:729:17)
...

@FG-avi
Copy link

FG-avi commented Apr 23, 2020

I also got different errors when I commented out a line of code. Currently, I can't have any empty lines between code and I can't comment out code either or storybook breaks when using mdx files.

@wooorm
Copy link
Member

wooorm commented Apr 23, 2020

Thanks for all your comments folks! We’re working on adding proper JSX parsing. 2.0.0 will solve many of the JSX parsing issues, including this one!

@atanasster
Copy link
Author

@wooorm great, looking forward to it. If you guys need some PRs or other help, pls give a shoutout.

@wooorm
Copy link
Member

wooorm commented Apr 23, 2020

2.0.0 will be all about syntax, which I’m working on. I’ve taken care of this issue (some more stuff to do though). If you’re interested, there are some issues marked as “help wanted” that are unrelated to syntax, that we could also use help on!

@shilman

This comment has been minimized.

@wooorm

This comment has been minimized.

@wooorm

This comment has been minimized.

@shilman

This comment has been minimized.

RobinWijnant added a commit to kodiak-packages/ventura that referenced this issue May 24, 2020
Prettier automatically adds new line characters in mdx.
The mdx formatter used by docz currently has a bug making builds fail
that have a new line character in  JSX. More on this bug:
mdx-js/mdx#767
@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 🐛 type/bug This is a problem and removed 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Dec 18, 2020
@wooorm wooorm added 👶 semver/patch This is a backwards-compatible fix 🗄 area/interface This affects the public interface labels Dec 18, 2020
@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/patch This is a backwards-compatible fix 🐛 type/bug This is a problem
Development

Successfully merging a pull request may close this issue.

8 participants