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

Fix support for comments #260

Merged
merged 1 commit into from
Sep 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/mdx/md-ast-to-mdx-ast.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,14 @@ const visit = require('unist-util-visit')

module.exports = options => tree => {
visit(tree, 'html', node => {
node.type = node.mdxType || 'jsx'
if (
node.value.startsWith('<!--') &&
node.value.endsWith('-->')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remark contains all comments as separate html nodes.

) {
node.type = 'comment'
} else {
node.type = node.mdxType || 'jsx'
}
})

return tree
Expand Down
5 changes: 5 additions & 0 deletions packages/mdx/mdx-ast-to-mdx-hast.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ module.exports = function mdxAstToMdxHast() {
type: 'export'
})
},
comment(h, node) {
return Object.assign({}, node, {
type: 'comment'
})
},
jsx(h, node) {
return Object.assign({}, node, {
type: 'jsx'
Expand Down
12 changes: 6 additions & 6 deletions packages/mdx/mdx-hast-to-jsx.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ function toJSX(node, parentNode = {}, options = {}) {
continue
}

if (childNode.type === 'jsx') {
childNode.value = childNode.value
.replace('<!--', '{/*')
.replace('-->', '*/}')
}

jsxNodes.push(childNode)
}

Expand Down Expand Up @@ -90,6 +84,12 @@ function toJSX(node, parentNode = {}, options = {}) {
return '{`' + node.value.replace(/`/g, '\\`').replace(/\$/g, '\\$') + '`}'
}

if (node.type === 'comment') {
return node.value
.replace('<!--', '{/*')
.replace('-->', '*/}')
}

if (node.type === 'import' || node.type === 'export' || node.type === 'jsx') {
return node.value
}
Expand Down
1 change: 0 additions & 1 deletion packages/mdx/test/fixtures/blog-post.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ I'm an awesome paragraph.
<Bar>hi</Bar>
{hello}
{/* another commment */}
<!-- one more comment -->
Copy link
Contributor Author

@silvenon silvenon Sep 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this line because, now that this is no longer treated as a comment, it causes an error when Babel tries to parse it.

</Foo>

```
Expand Down
29 changes: 24 additions & 5 deletions packages/mdx/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,20 +86,39 @@ COPY start.sh /home/start.sh

it('Should support comments', async () => {
const result = await mdx(`
A paragraph
<!-- a Markdown comment -->
A paragraph

Some text <!-- an inline comment -->

\`\`\`md
<!-- a code block Markdown comment -->
<!-- a code block string -->
\`\`\`

<div>
{/* a nested JSX comment */}
<!-- a nested Markdown comment -->
<!-- div content -->
Copy link
Contributor Author

@silvenon silvenon Sep 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would technically cause a Babel error (as mentioned above), but I included it in the test to ensure that it doesn't turn into a comment.

</div>

<!-- a comment above -->
- list item
<!-- a comment below -->

--> should be as-is

<MyComp content={\`
<!-- a template literal -->
\`}
`)
expect(result).toContain('{/* a Markdown comment */}')
expect(result).toContain('<!-- a code block Markdown comment -->')
expect(result).toContain('{/* an inline comment */}')
expect(result).toContain('<!-- a code block string -->')
expect(result).toContain('{/* a nested JSX comment */}')
expect(result).toContain('{/* a nested Markdown comment */}')
expect(result).toContain('<!-- div content -->')
expect(result).toContain('{/* a comment above */}')
expect(result).toContain('{/* a comment below */}')
expect(result).toContain('--> should be as-is')
expect(result).toContain('<!-- a template literal -->')
})

it('Should not include export wrapper if skipExport is true', async () => {
Expand Down