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

MDX renderer fails to apply custom delete (del) component #801

Closed
markmichon opened this issue Oct 5, 2019 · 5 comments
Closed

MDX renderer fails to apply custom delete (del) component #801

markmichon opened this issue Oct 5, 2019 · 5 comments
Labels
good first issue 👋 This may be a great place to get started! help wanted 🙏 This could use your insight or help 🐛 type/bug This is a problem 🙆 yes/confirmed This is confirmed and ready to be worked on

Comments

@markmichon
Copy link

Custom delete component fails to render

First ran into this over in system-ui/theme-ui#401 where applying a custom delete component failed to render when passed into MDXRenderer's component prop. MDX rendered a del, but not the custom one.

I've tried tracking down exactly where the issue happens, but only managed to get a few failing tests going in mdx-js and mdx-js/react.

Steps to reproduce / Expected / Etc

Here's a commit with the two failing tests, or see the input/output below.

For mdx core:

const components = {
  delete: ({children}) =>
    React.createElement('del', {style: {color: 'crimson'}}, children)
}
it('renders delete component from context', async () => {
  const result = await renderWithReact(`Hello, ~~world~~ MDX!`, {components})

  expect(result).toContain(
    '<p>Hello, <del style="color:crimson">world</del> MDX!</p>'
  )
})
Expected substring: "<p>Hello, <del style=\"color:crimson\">world</del> MDX!</p>
"
    Received string:    "<p>Hello, <del>world</del> MDX!</p>"

Similar deal with @mdx-js/react:

const CustomDelete = props => <del style={{color: 'crimson'}} {...props} />

it('Should render delete as del', () => {
  const result = renderToString(
    <MDXProvider components={{delete: CustomDelete}}>
      <Fixture />
    </MDXProvider>
  )

  // CustomDelete is rendered
  expect(result).toMatch(/style="color:crimson"/)
})
Expected pattern: /style="color:crimson"/
    Received string:  "<h1></h1><h2></h2><p>h3, h4</p>"

I suspect it's further up the chain somewhere, so maybe someone with better knowledge of the ecosystem can hunt it down or point me in the right direction. Thanks!

@markmichon markmichon added 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Oct 5, 2019
@markmichon markmichon changed the title MDXRenderer fails to apply custom delete (del) component MDX renderer fails to apply custom delete (del) component Oct 5, 2019
@johno johno added 🙆 yes/confirmed This is confirmed and ready to be worked on 🐛 type/bug This is a problem and removed 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Oct 5, 2019
@johno
Copy link
Member

johno commented Oct 5, 2019

Thanks for reporting! This definitely looks like a bug. Do you mind opening up a PR with your failing tests @markmichon? That way we could use that as a starting point to get a fix in.

@johno johno added help wanted 🙏 This could use your insight or help good first issue 👋 This may be a great place to get started! labels Oct 5, 2019
@Luiginator
Copy link
Contributor

Luiginator commented Oct 22, 2019

@markmichon @johno I found some issues in the test cases and created a PR that proofs that the del works also with custom components: #825

I am not sure if we really need the tests in the repo this was just a proof that it works.

@markmichon
Copy link
Author

@Luiginator There's a good chance that my tests may be flawed since I'm not entirely sure where the problem is, but the issue is that mdx (or somewhere up the chain) is expecting delete as the key, not del. See https://mdxjs.com/getting-started#table-of-components . Your tests look to expect del to del, rather than delete to del.

The expected outcome would be passing in a component mapped to delete, and outputting markup as del. Such as: ({delete: CustomDelete }) => <del ... />

@Luiginator
Copy link
Contributor

@markmichon Ah sorry you are right. I did not see the documentation you linked, thank you!

@johnletey Is there any chance that the documentation for this is wrong? There does not exist a delete tag in html there is just a 'del' tag. Given that all other components use the original tag and would be invalid html I would expect 'del' and not 'delete'. Also 'del' is currently working and 'delete' is not.

@wooorm
Copy link
Member

wooorm commented Oct 19, 2021

There’s a new release candidate out for v2. See more on the website for that: https://v2.mdxjs.com.
One feature that it includes is a one-to-one mapping of HTML tag names to components. That means del is now supported. More info on the Table of components: https://v2.mdxjs.com/table-of-components/.

@wooorm wooorm closed this as completed Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue 👋 This may be a great place to get started! help wanted 🙏 This could use your insight or help 🐛 type/bug This is a problem 🙆 yes/confirmed This is confirmed and ready to be worked on
Development

No branches or pull requests

4 participants