Add Option to override built-in HTML elements

[jlaramie]

There is no way to tell @mdx-js/mdx to use passed in components when overriding native built-in HTML elements.

This is a big pain if you want to provide some custom handling for certain native HTML elements.

I looked into disableParentContext and it does not provide this feature at all. disableParentContext just prevents the use of components from a parent provider but does not control the code output in a way that would allow overriding native HTML elements.

I added a test which shows what I look to accomplish

Related To:

• Override jsx built-in components #2050
• Don't replace literal HTML elements with MDXProvider #821

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
mdx	✅ Ready (Inspect)	Visit Preview	May 29, 2022 at 8:15PM (UTC)

[codecov-commenter]

Codecov Report

Merging #2052 (835d50f) into main (63fd208) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main     #2052   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines         2308      2322   +14     
=========================================
+ Hits          2308      2322   +14     

Impacted Files	Coverage Δ
packages/mdx/lib/core.js	100.00% <100.00%> (ø)
packages/mdx/lib/plugin/recma-jsx-rewrite.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63fd208...835d50f. Read the comment docs.

[jlaramie]

Readme still needs to be updated to include the new option. Just trying to figure out which readme files need editing.

[wooorm]

Hey! My comment below is somewhat negative. That has to do with what the technical properties of this PR. It has no bearing on whether or not you are running into a problem.

• This behavior is intentional. If you want to pass custom components, either a) import the component manually, or b) use a component name like any other component (e.g., <Img>) to differentiate between the components written in markdown syntax (![x](y)), which often have a certain way to render that is different compared to explicit JSX tags (such as <img />)
• The docs for this are incorrect. This option isn’t about MDXProvider, which is not required for MDX to work or to pass components. This option is about passing components.
• Why should this be an option instead of a plugin. Most custom needs that people have are plugins. Other people have also solved this with plugins. Why should this be something that’s maintained here?

[jlaramie]

Hey! My comment below is somewhat negative. That has to do with what the technical properties of this PR. It has no bearing on whether or not you are running into a problem.

All good. I know how it can be with dealing with pull requests and I did not create an issue to talk about this in.

• This behavior is intentional. If you want to pass custom components, either a) import the component manually, or b) use a component name like any other component (e.g., <Img>) to differentiate between the components written in markdown syntax (![x](y)), which often have a certain way to render that is different compared to explicit JSX tags (such as <img />)

I understand this is "intentional" now, but V1 specifically had the opposite handling so it's a pretty big breaking change that is barely even mentioned in the documentation (I couldn't find anything mentioning it until it was pointed out).

This is why I created a new pull request and set it as an passable option instead of overriding the now intentional behavior.

• Why should this be an option instead of a plugin. Most custom needs that people have are plugins. Other people have also solved this with plugins. Why should this be something that’s maintained here?

You are probably right in that this could be a plugin that essentially duplicates some of the processing happening in recma-jsx-rewrite.js but in my opinion not being able to override native HTML elements without an additional plugin is confusing to developers and it is unnecessary to have to develop and maintain another plugin for what equates to a single line change to maintain.

Quoted from #821 (comment)

I would love a way to be able to tell MDX "please only transform markdown tags, not literal HTML tags" so that literal HTML can be used as an escape hatch.

Issue 821 refers to the want to be able to tell MDX if you do or do not want to override literal HTML tags but the changes it presented completely removed the functionality with no way to add it back without setting up another plugin to parse the tree.

[jlaramie]

@wooorm I've gone ahead and added some documentation that will maybe make this clearer

https://mdx-git-fork-jlaramie-override-jsx-components-mdx.vercel.app/migrating/v2/#mdx-jsmdx-sandboxing
https://mdx-git-fork-jlaramie-override-jsx-components-mdx.vercel.app/packages/mdx/#optionsoverridebuiltin

[wooorm]

but V1 specifically had the opposite handling so it's a pretty big breaking change that is barely even mentioned in the documentation (I couldn't find anything mentioning it until it was pointed out).

I have found that many people did not expect the previous behavior and I have heard very few people that miss it. Other than that expectation, there’s also the fact that: it was very hard to undo the replacement, while it is very easy to add <Image /> if you want that.

You are probably right in that this could be a plugin that essentially duplicates some of the processing happening in recma-jsx-rewrite.js but in my opinion not being able to override native HTML elements without an additional plugin is confusing to developers and it is unnecessary to have to develop and maintain another plugin for what equates to a single line change to maintain.

I have no clue what you are doing. Perhaps you can share why it is impossible for you to call things <Image /> in your MDX files. I find this a very obvious and acceptable solution. I don’t understand why it doesn’t work for your case.

The plugin would be extremely simple: it would essentially be visit(tree, (node) => { delete node.data._mdxExplicitJsx })

From my perspective, the previous results have lead many people astray. I am choosing between to complex states.

I also have to choose between more complex APIs with more options, and less complex APIs. Especially if options are niche.

[jlaramie]

This behavior is intentional. If you want to pass custom components, either a) import the component manually, or b) use a component name like any other component (e.g., <Img>) to differentiate between the components written in markdown syntax (![x](y)), which often have a certain way to render that is different compared to explicit JSX tags (such as <img />)

Another point here is that unless all the mdx is written with implicit import statements a regular mdx viewer cannot handle custom components which are being passed into it via components property.

So this works in a regular renderer

import { Img } from "./Img.jsx";

<div>
  <Img src="my-image.gif" /> 
</div>

This does not work in a regular renderer since Img isn't explicitly defined

<div>
  <Img src="my-image.gif" /> 
</div>

This works and can be useful for a rough preview outside of a build system even though the output doesn't look 100% like what the final rendered output would look like.

<div>
  <img src="my-image.gif" /> 
</div>

Another example is how when I was editing the md files to add documentation, I was able to run it through a mdx preview vscode plugin, even though the rendered output is not the same as what is deployed to vercel. This was still useful to me in order to check for some common mistakes when formatting the additional documentation. Had the md files used custom components isntead of native ones, the mdx preview would have failed unless there were implicit import statements at the top.

[jlaramie]

I have no clue what you are doing. Perhaps you can share why it is impossible for you to call things in your MDX files. I find this a very obvious and acceptable solution. I don’t understand why it doesn’t work for your case.

See my notes above about previewing the mdx in different environments

The plugin would be extremely simple: it would essentially be visit(tree, (node) => { delete node.data._mdxExplicitJsx })

This does indeed seem easy and maybe some documentation in the migration docs would have led me to this solution first.

[wooorm]

Hmm, I find the previewer issue rather interesting, but I’m not sure I really buy into it.
I’m leaning towards the idea that the previewer is lacking some features which can improve previewing.
Maybe this could he raised with them: that it supports a place to import components from.
I can also envision that they’d instead render components they don’t know (Youtube? etc) as code instead of as an error.

This does indeed seem easy and maybe some documentation in the migration docs would have led me to this solution first.

A lot of cool things can be done with plugins. They became extra powerful in v2. They are collected here: https://mdxjs.com/docs/extending-mdx/#list-of-plugins. Perhaps you want to make one, list it there, and we can work on adding it to migration too?

[jlaramie]

It did take me some playing around to find which plugin had all the information necessary to do this

{
  rehypePlugins: [
    () => (tree) => {
      visit(tree, "mdxJsxTextElement", (node) => {
        if (node.data) {
          delete node.data._mdxExplicitJsx;
        }
      });
    },
  ]
}

[jlaramie]

Obviously this isn't my project so do with it what you want. I was just trying to add back a feature that got removed even though the ticket associated to the change wanted to make it optional. I still think there is are plenty of really good use cases for this and have used mdx-js in this manner for quite some time so it's a bit of a bummer that apparently the feature I relied on is unwanted.

I'd suggest at least updating the migration documentation with the above plugin or something similar in order to avoid confusion.

I'm more than happy to continue working on this pull request in order to get documentation and naming conventions to your standard, but if you don't want this change, go ahead and close this out I guess.

Thanks for your time here and the time you put into this package.

[wooorm]

Heya!

I still think there is are plenty of really good use cases for this

I do not understand your use case. If you want say custom images, use a component with a capital <Img />, which is explicit.
Overwriting markdown things is meant for overwriting the things you write in markdown.
If you want to overwrite both images in markdown and use a component, you can pass both and be explicit: img: Img, Img.

so it's a bit of a bummer that apparently the feature I relied on is unwanted.

People ran into it and didn’t understand it and it caused bugs. Having the feature but reverting it is complex to reason about (e.g., a sandbox component). Not having the feature but adding it is easy and explainable: by using a capital, or a plugin.

I'd suggest at least updating the migration documentation with the above plugin or something similar in order to avoid confusion.

There are not many people that run into your problem and don’t like it, so I’m not sure it should be thoroughly documented in the migration docs. That it changed is explained in https://mdxjs.com/migrating/v2/#update-mdx-content:

We now “sandbox” components, for lack of a better name. It means that when you pass a component for h1, it does get used for # hi but not for <h1>hi</h1>

I’d be interested in hearing what you propose to change there, but I find it fine.

---

Closing because I currently don’t believe this should be an option

[huv1k]

I would love to see this feature 😅 I just spend 2 hours trying figure out what is best approach, but this would be super useful 🙈

[wooorm]

Please see the thread. It is unclear why the good alternatives are not enough. You can use <Img> or so and it is explicit and it works.

[ChristianMurphy]

Locking this thread as the resolution is to add a plugin.

#2052 (comment) and #2052 (comment) demonstrate how that plugin can be added