rehypejs / rehype-react

plugin to transform to preact, react, vue, etc
https://unifiedjs.com
MIT License
390 stars 27 forks source link

React warning for whitespace in tables #28

Closed stevemk14ebr closed 3 years ago

stevemk14ebr commented 3 years ago

I'm using the plugin like this:

 const markdownConverter = Remark().use([RemarkParse, RemarkGFM,
        [RehypeUnwrap, { disallowedElements: ["p", "pre"], unwrapDisallowed: true }],
        [RehypeReact, { createElement: React.createElement, Fragment: React.Fragment }]]);
const htmlmarkdown = markdownConverter.processSync(markdown).result;

Given the following markdown:

"|File Name|File Type|Size|Compile Time|\n| ----------- | ----------- | ----------- | ----------- |\n|nothing|nothing|nothing|nothing|"

The generated table has a bunch of newline text nodes as children, which causes react to throw the error "Whitespace text nodes cannot appear as a child of < tbody >" and more for different type like tr and th.

Example of the spurious nodes: image

I'm not sure how to troubleshoot this or disable these

ChristianMurphy commented 3 years ago

@stevemk14ebr a few things:

  1. you need remark-rehype to turn markdown/mdast into html/hast
  2. Not sure what rehype unwrap is or where it came from, it doesn't appear to be on npm

When used with remark rehype and without the unwrap plugin no error is present. https://codesandbox.io/s/remark-rehype-debug-forked-dtzkw?file=/src/index.js

stevemk14ebr commented 3 years ago

Hi your demo is using remark-stringify which does not appear to have this issue. As far as i can tell the issue is with rehype-react. I am using remark-rehype just forgot to include that in the snippet, rehype unwrap is a custom thing i made it's not relevant. Here is a reproduction of what i'm seeing, can this be re-opened please?

https://codesandbox.io/s/remark-rehype-debug-forked-21j7e

interestingly the issue here is not just \n like i was seeing. There's quite a few empty text nodes at each level of the tree: image

ChristianMurphy commented 3 years ago

Thanks for clarifying! I'm not sure this is coming from rehype-react inspecting the AST at each step, the spaces appear to come into existence after remark-rehype https://codesandbox.io/s/rehypereact-bug-forked-sjuse

stevemk14ebr commented 3 years ago

oh cool thanks for showing how to dump the tree easily! I agree it does seem to be the case, it seems like it's trying to format the html it generates nicely by including some extra newlines and whitespace but these nodes are not removed and so cause issues like this. It seems these ARE present with the stringify plugin too, however since you set the html directly and bypass react the whitespace errors are not present (as react's parser generates these)

wooorm commented 3 years ago

The newlines are supposed to be there at the remark-rehype phase. React for some unnecessary reason doesn’t like newlines though. A PR similar to https://github.com/remarkjs/remark-react/commit/228fd8e6b72172d0867fe113efa52291c45be691 for this project would be a solution

stevemk14ebr commented 3 years ago

So that I understand, the remark-hype plugin used to not emit these newlines but that was removed a while ago? Now, this rehype-react plugin should be responsible for removing these newlines (since they're only invalid in this context)?

wooorm commented 3 years ago

No, remark-rehype has always emitted them, and it adheres to how markdown works: with newlines. The fact that React warns about them, is more an issue with React. For some reason, in most cases, React does not care about the newlines, but in certain cases it does warn. I’m not exactly what your setup is to cause the warning, but indeed: we can remove them in this plugin

stevemk14ebr commented 3 years ago

it's the table type for me that causes issues, but i'm sure react cares about other html types

wooorm commented 3 years ago

Released! https://github.com/rehypejs/rehype-react/releases/tag/6.2.1

TheComp44 commented 3 years ago

I think the issue should be reopened, since the fix doesn't work for all whitespace nodes. Just filtering for "\n" is too specific as whitespace nodes could also consist of an arbitrary number of newlines, spaces and tabs.

When using rehype-parse on an html document with a table, I get the same react warning since the test misses on tables with indentations. For example:

<table>
    <tbody>
        <tr>
            <td>Test</td>
        </tr>
    </tbody>
</table>