Preserve whitespace in code blocks #226

silvenon · 2018-08-18T14:52:15Z

I'm trying to write a test that reproduces #221. The idea is to mount the component exported by the fixture and use textContent to check if whitespace in the code block is preserved.

I'm having difficulties trying to load the module compiled by webpack as a React component and mounting it. Webpack gives me an object with a source text, but I'm not sure how to parse it as a Node module. If someone has an idea, let me know. 😅

P. S. I'm not 100% certain that this is really an MDX issue, but the fact that I can't reproduce it without it suggests that it is.

vercel · 2018-08-18T14:52:17Z

This pull request is automatically deployed with Now.

To access deployments, click Details below or on the icon next to each push.

silvenon · 2018-08-18T15:28:00Z

I realized I could test this easily with runtime, so nvm, I think I got it. 😉

silvenon · 2018-08-18T15:39:54Z

Ok, I managed to reproduce this in a test! Now MDX runtime also accepts mdPlugins, hastPlugins and compilers. Feedback on this API change would be welcome.

hugmanrique · 2018-08-24T15:30:10Z

So would moving the prism plugin to compilers fix the issue? What's the main difference between mdPlugins and this new option?

silvenon · 2018-08-24T18:51:18Z

hastPlugins operate on rehype tree, while compilers operate on JSX strings. Notice that between applying these two sets of plugins the tree is converted to JSX. @mapbox/rehype-prism can only work on rehype, so I think it has to stay in hastPlugins.

I get lost pretty easily when working with unified because I haven't mastered it yet, but I'll continue trying to figure out where this is failing.

silvenon · 2018-08-26T15:47:51Z

I figured it out! In the process of converting to JSX there was an exception with newlines introduced by #180. I added an exception to that exception (exceptionception) and converted newlines to JSX text nodes only for code inside <pre> tags.

This works, I've been using it locally.

@ifyoumakeit because you have a deeper understanding of that part of the code, could you take a look at this?

ifyoumakeit · 2018-08-26T17:19:41Z

I can definitely can take a look!

ifyoumakeit · 2018-08-27T02:13:01Z

packages/mdx/mdx-hast-to-jsx.js

+    children = node.children.map(childNode => {
+      const childOptions = {
+        ...options,
+        preserveNewlines: preserveNewlines || node.tagName === 'pre',


👍 I think this makes sense, as long as there's information about this default behavior somewhere.

ifyoumakeit

Looks good! ⛱

I noted this in a comment, but I would just make sure this is well documented and not buried in the code.

alexkrolick · 2018-08-27T03:07:37Z

Nice, this was super annoying: alexkrolick/mdx-observable#1 (https://github.com/alexkrolick/mdx-observable/blob/086f477b38885e21b616b5e2b45518a2bb9d3245/demo/counter-child-function.mdx#L10-L20)

silvenon · 2018-08-27T08:57:29Z

@ifyoumakeit thanks a lot! I’ll make sure to at least add comments.

@alexkrolick could you be more specific? What problem would these changes solve for you?

alexkrolick · 2018-08-27T09:05:25Z

Actually I might be me misinterpreting the problem. I thought this would fix the fact that newlines inside render functions are interpreted as MD blocks instead of as JS code to evaluate. You can see how the formatting in my link requires adding comments or semicolons anywhere you want a blank line.

silvenon · 2018-08-27T17:05:13Z

This PR unfortunately won't take care of that, it's for fenced code blocks only. We're aware of the issue you mentioned, though.

The advantage of the toContain() matcher is apparent on test failure; Jest will print out the actual vs. expected, instead of only giving you the information that it failed.

In order to avoid issues like React warnings caused by text nodes in tables, we didn't convert text nodes consisting solely of one newline into text nodes. This caused some newlines inside fenced code blocks to be lost because, unlike in HTML, in React we have to explicitly create text nodes, which is important whitespace-sensitive tags like <pre>. These changes convert all newlines in <pre> tags into text nodes. Fixes #221.

silvenon · 2018-08-27T17:47:19Z

Done! Polished it up a bit and added more comments to clarify what's going on.

I also updated all mdx tests to use the toContain() matcher because it really improves debugging experience in tests, for example you get transparent errors like this:

    Expected string:
      "export default ({components, ...props}) => <MDXTag name=\"wrapper\"  components={components}><MDXTag name=\"p\" components={components}><MDXTag name=\"inlineCode\" components={components} parentName=\"p\">{`<div>`}</MDXTag></MDXTag></MDXTag>"
    To contain value:
      "<MDXTag name=\"inlineCode\" components={components} parentName=\"p\">{`<b>`}</MDXTag>"

@johno ready when you are. 😉

johno · 2018-09-04T14:44:08Z

Thanks all, this is amazing!

ifyoumakeit · 2018-09-04T21:06:15Z

packages/mdx/test/index.test.js

@@ -97,7 +103,7 @@ it('Should render elements without wrapping blank new lines', async () => {
  | :--- | :---- |
  | Col1 | Col2  |`)

-  expect(result.includes('{`\n`}')).toBe(false)
+  expect(result).not.toContain('{`\n`}')


devongovett · 2018-09-07T17:49:54Z

Can we get this released?

johno · 2018-09-07T17:54:33Z

Now published as 0.15.1, apologies on the delay.

silvenon changed the title ~~[WIP] Preserve whitespace in code blocks~~ Preserve whitespace in code blocks Aug 26, 2018

ifyoumakeit reviewed Aug 27, 2018

View reviewed changes

ifyoumakeit approved these changes Aug 27, 2018

View reviewed changes

silvenon added 2 commits August 27, 2018 19:18

Use toContain() matcher for mdx tests

225838d

The advantage of the toContain() matcher is apparent on test failure; Jest will print out the actual vs. expected, instead of only giving you the information that it failed.

silvenon mentioned this pull request Aug 28, 2018

Using React components in Markdown source gatsbyjs/gatsby#312

Closed

Merge branch 'master' into code-blocks-whitespace

abf9291

johno merged commit e883828 into mdx-js:master Sep 4, 2018

ifyoumakeit reviewed Sep 4, 2018

View reviewed changes

silvenon deleted the code-blocks-whitespace branch September 5, 2018 13:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preserve whitespace in code blocks #226

Preserve whitespace in code blocks #226

silvenon commented Aug 18, 2018

vercel bot commented Aug 18, 2018

silvenon commented Aug 18, 2018

silvenon commented Aug 18, 2018 •

edited

Loading

hugmanrique commented Aug 24, 2018

silvenon commented Aug 24, 2018 •

edited

Loading

silvenon commented Aug 26, 2018 •

edited

Loading

ifyoumakeit commented Aug 26, 2018

ifyoumakeit Aug 27, 2018

ifyoumakeit left a comment

alexkrolick commented Aug 27, 2018 •

edited

Loading

silvenon commented Aug 27, 2018

alexkrolick commented Aug 27, 2018

silvenon commented Aug 27, 2018 •

edited

Loading

silvenon commented Aug 27, 2018 •

edited

Loading

johno commented Sep 4, 2018

ifyoumakeit Sep 4, 2018

silvenon Sep 5, 2018

devongovett commented Sep 7, 2018

johno commented Sep 7, 2018

Preserve whitespace in code blocks #226

Preserve whitespace in code blocks #226

Conversation

silvenon commented Aug 18, 2018

vercel bot commented Aug 18, 2018

silvenon commented Aug 18, 2018

silvenon commented Aug 18, 2018 • edited Loading

hugmanrique commented Aug 24, 2018

silvenon commented Aug 24, 2018 • edited Loading

silvenon commented Aug 26, 2018 • edited Loading

ifyoumakeit commented Aug 26, 2018

ifyoumakeit Aug 27, 2018

Choose a reason for hiding this comment

ifyoumakeit left a comment

Choose a reason for hiding this comment

alexkrolick commented Aug 27, 2018 • edited Loading

silvenon commented Aug 27, 2018

alexkrolick commented Aug 27, 2018

silvenon commented Aug 27, 2018 • edited Loading

silvenon commented Aug 27, 2018 • edited Loading

johno commented Sep 4, 2018

ifyoumakeit Sep 4, 2018

Choose a reason for hiding this comment

silvenon Sep 5, 2018

Choose a reason for hiding this comment

devongovett commented Sep 7, 2018

johno commented Sep 7, 2018

silvenon commented Aug 18, 2018 •

edited

Loading

silvenon commented Aug 24, 2018 •

edited

Loading

silvenon commented Aug 26, 2018 •

edited

Loading

alexkrolick commented Aug 27, 2018 •

edited

Loading

silvenon commented Aug 27, 2018 •

edited

Loading

silvenon commented Aug 27, 2018 •

edited

Loading