-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Docs: Missing explicit docs and workaround for overriding literal HTML elements in MDX #2503
Comments
Heyyyyy! This is intentional, because it isn’t needed, you can do:
And then: <Video /> Why is this one character change, which is inspired by how JSX itself works (if you write
|
I haven't yet collected all of the reasons why others have said they wanted this, but I'll list some reasons why overriding
I can understand if there are technical reasons why MDX does not want to support these use cases - are there technical reasons for this? The original workaround suggested a plugin that no longer works. So if there's a technical reason for it not working or a new version of the workaround for changes in MDX, that would be interesting. Regardless of whether there is a new workaround or not, I do think the whole flow should be more explicitly documented, with a few examples of what HTML tags are not replaced. Here is a detailed list of suggestions, based on my original list above and your questions above, but reordered to match the order of the page: 4) Using MDX -> Components section - text description of what MDX does
-There is one special prop: `components`. It takes an object mapping component names to components. Take this example:
+There is one special prop: `components`. Pass an object containing either / both of:
+
+1. Component name keys to components values eg. `Video: () => { ... }`
+2. HTML element names to components eg. `img: Image`
+
+MDX will use this object to replace any occurrences in your MDX file with the component passed as the value.
+
+Take this example: This may have issues which require further clarification eg. that these key/value pairs above only apply to MDX with HTML. 3) Using MDX -> Components section - Resulting HTML of
|
Thanks for sharing @karlhorky! This comes up often enough I'd be okay with there being some documentation. With a sizable section on alternatives and why you may not want to do this (most people), before offering a path to the persistent who have a super specific use case. |
The only people I am aware of that brought this up was people migrating.
This is not a goal, as copy/pasting HTML into MDX will not work. Think about You are arguing about
I argue this the other way around: if you are going to use a
Perhaps I am not 100% on what you mean, but I think this preference is wrong, it doesn’t work due to
Which tags are (not) replaced?
I think this is getting very close to what is at the end of that section already:
The examples in these docs don’t focus on input -> output, for several reasons.
For me “keys in x” already implies object, I don’t think this change is needed?
I don’t think your change is an improvement.
I don’t think the change is correct. There is a previous sentence: it does include all HTML equivalents for the things you write with markdown.
Well, a) I really don’t consider this a “caveat”, and, b) I believe this duplicating what comes after:
This “caveat” is a fundament of JSX: how to differentiate between a reference to a variable and a literal tag name. Perhaps we can illustrate that section by following it with an example of this sort:
…and then with:
…and then show the generated HTML? Perhaps even rendered (red, mdx link color, blue)?
It’s not that
I dunno, the second word is “equivalent”. And then there’s a whole sentence.
That’s because
No “disallowlist” is needed because there’s only “allowlist”s. There are cases where
I described this in more depth already but in short: how to use references in JSX (
Ok, I think this one is fine.
Same as the above. I don’t think output is needed; This section isn’t about the input, or the components, or the output. It’s about whether |
We don't need to go over the reasons for why to use I have a gut feeling that the rebuttals of my reasons are not necessarily taking in the complete picture, but then again, it's possible that you just have more experience than me with these things and have already gone down these roads far enough to see more. Given more usage of Added Explicitness, Complete Examples
I think it's all the same answer for these: the examples and additional explicitness are meant to be aids to help make the concepts concrete for learners. My reasoning is in my experience of seeing that explaining one thing one way, in terse writing and only partially (even if it can be explained eg. in this issue as being a full explanation) will lead to misunderstandings and half-formed knowledge. To the "partially" point:
Restructuring / Added Explicitness with Misunderstanding Potential
Some sections and bullets feel like they have high potential for misunderstanding, including the bullet about HTML-equivalent elements. Restructuring the bullet text to start with a different word than "HTML" and adding a caveat are 2 ways to be explicit to avoid misunderstandings. Alternative "caveat" suggestion
An example would definitely help and at least partly achieve many of the things I'm aiming to with these suggestions 👍 Complete Playground
I don't mean to suggest a full playground here - I'm suggesting showing the output, which is the step in between which these examples are missing. Suggestions to Use Components
I don't think there's a suggestion anywhere that Having an explicit, blessed suggestion for using But maybe this actually is even bigger than I'm thinking and needs its own section, with use cases:
This actually is kind of similar to what is above in your suggestion in "Alternative "caveat" suggestion", wonder if these could be combined... 🤔 |
There is hierarchy to this solution space There are a lot of ways to solve an issue like this. If you want more features/customizations in how a video is rendered. If for some reason you choose not to use the platform or the framework, then yes, using MDX advanced internals you can change how the content works. You very well may have a valid use case for it, I appreciate and respect that.
Then use HTML, don't turn standard HTML elements into something custom. |
This feels more appropriate |
Removed the asterisk in 90f509b.
Done in 044e8b2. Further responses to most of your comments.
This sounds contradictory. You haven’t used this much but we are the ones missing the complete picture. 😅
I think there’s also a trade off here. My argument goes similar to how I argued for science papers somewhere else: one does not get far if one constantly has to explain every concept in detail, so that it can be understood by beginners, that don’t read the surrounding text. More text costs every reader too.
These are broad statements. There are of course cases where they can be implemented. But there might be cases where I don‘t think it’s needed. I’d appreciate it when they can be made concrete: which concepts are explained a singular way? I may be able to point to other docs that explain it other ways. Or more completely. Particularly regarding the “will lead to misunderstandings and half-formed knowledge” part: I believe this to be inevitable for learners. Not even for just for the beginners, that grow (and can only grow due to it). But anyone reading a sentence once will have misunderstandings and half-formed knowledge. Anyone only reading theory and not putting it into praxis will misunderstand.
I fail to see this. I don’t see a problem here nor an improvement in the suggestion: these values for keys do come from the HTML spec. Not RSS or SVG or JS or CSS. Perhaps reading through the earlier document “What is MDX?” might be useful in clearing up what HTML syntax / JSX syntax mean, in relation to MDX?
The output is missing for all these examples, in this document and in “What is MDX?”, because it doesn’t matter what the output is here. So I am extrapolating what you are asking for: if here, why not everywhere? If output, why not a package.json, or entire playgrounds?
It’s not there because I don‘t think that question is for that spot. There is an explanation of the difference between
Sure. How to use MDX at scale. How to enjoy writing MDX. MDX patterns. Stuff like that could all be its own guides. They are more tied to personal opinions. And presumably also more tied to a particular platform or a framework. So where it goes depends on what you want to suggest! |
Coming back to the original issue, I think this can be closed.
Do please comment when you want to add something important: comments still work on closed issues. But also consider my goal is to move to concrete improvements and discussions. |
Ok, thanks! The commit 044e8b2 does achieve 2 things in my perspective:
I think what's missing is:
I opened a PR for 1 (I originally suggested HTML, but I kept JSX for the output), 2, 3 and 4 for first feedback: Because the rest of the points seem lower likelihood to be accepted, I have held off with creating a PR for those. |
There are a lot of different outputs.
I'm a bit cautious about starting a dumping ground of all the things MDX doesn't do.
Improved formatting is welcome!
I'd second Titus' point here.
Not quite what I was getting across here.
It feels like this may be best at the type level first. |
This is intentionally omitted. I mentioned in previous comments why I don’t believe the output is not relevant here. As Christian mentions, to MDX, there are also infinite outputs. And, it’s explained in a preview and prose already (next point). Christian mentions this too.
It is explicitly described in the paragraph after.
Perhaps I don’t know what you mean. The following paragraph does describe visual formatting. Do you mean using those colors and borders in the text as well? That doesn’t feel good. When you highlight everything, nothing is highlighted.
I don’t believe it’s needed here: I am sure you are aware of “show, don’t tell”; and repetition has also already been noted in the discussions too. Showing and then telling seems fine. I don’t see what tell then show then tell again adds.
I don’t really see it. Strengths and weaknesses. Here. MDX supports markdown and JSX. What you subjectively think is a great idea in your current stack has little to do with MDX. Perhaps you are looking for “What is MDX”. I am open, like Christian, to a guide or similar somewhere on when to choose Markdown/HTML/custom elements/plugins/transforms/syntax extension.
Christian’s answer is sufficient here I believe.
Anything is possible there. |
With "visual formatting" I mean making the prose visually structured and designed like the code it's describing, making it easier to scan and match up with the code while looking back and forth between the code and prose. Since there are 3 JSX elements, I added 3 bullet points. The idea behind it: the visual shape of the prose and code in writing and docs is important for scanning, making it inviting and not burying info (also which word starts a line or sentence).
Right, I think this is where a key disagreement (which appears unresolvable) is - in my (strong) opinion coming from lots of observation, repetition is very useful for beginners, and has only the downside of taking up space, which I'm trying to minimize with keeping it short. Terseness and purity are good for the authors (less to produce) and maybe for making it more visually minimal, but the downsides are bigger - that it doesn't get the point across. Probably my opinion and suggestions and style for teaching beginners is just incompatible with the MDX style, so I think me contributing less is the best choice here. It feels insurmountable to introduce change to the current MDX style. I'll still do small PRs as they come up but I'll keep in mind that the method of teaching is probably not going to change.
Interesting, what are the biggest, most popular uses and consumers of MDX? I thought for sure it would be the React ecosystem, haven't seen it in big projects elsewhere. I don't really have a strong opinion on where it should go in the docs, but at the point that the concept of component replacement is being introduced (which is where I'm suggesting it go), I would expect a small overview of those "best practices" from the MDX team to be introduced (maybe linked to another short guide somewhere). As of now, these best practices are buried in issues. So even putting it somewhere non-ideal would be an improvement in visibility. |
Sure. There is already repetition. By this logic 5 or 20 repetition is an improvement.
I don’t think that’s relevant. Not only the biggest mass of beginners deserves docs on this website. You are free to teach only React beginners. There are React-specific docs here. It’s also used in many examples. But it’s not the focus. By this logic we should drop React on this website for PHP or Rails. |
Initial checklist
Affected packages and versions
@mdx-js/react@3.0.1, next@14.2.4, react@^18
Link to runnable example
No response
Steps to reproduce
app-dir-mdx
example from Remcoexperimental.mdxRs
fromnext.config.js
video: () => <div>video replacement</div>
inmdx-components.tsx
<video />
inapp/message.mdx
<video />
appearing in HTML in DevTools, instead of the<div>
components
do not override the built-in react components kentcdodds/mdx-bundler#160components
do not override the built-in react components kentcdodds/mdx-bundler#160 (comment)visit()
video elements of typemdxJsxFlowElement
? remcohaszing/rehype-mdx-import-media#2 (comment)rehypePlugin
suggested by @wooorm)<video />
still appearing in HTML in DevTools, instead of the<div>
Or, to skip steps 1-6, a reproduction repo:
This has occurred because some users have seen the "overriding literal HTML in MDX" as unexpected behavior (as in @adamwathan's original #821). Other users (including myself) have found it unexpected that these HTML elements are not being overridden, especially in the case of HTML elements which have no Markdown syntax equivalent, such as
<video>
, where it would be amazing to be able to userehype-mdx-import-media
with<video>
out of the box without any additional plugins:visit()
video elements of typemdxJsxFlowElement
? remcohaszing/rehype-mdx-import-media#2 (comment)Expected behavior
The Using MDX page (specifically the Components section and the MDX provider section) should include:
components
prop does (not onlyexample.mdx
andexample.jsx
, but also the resulting HTML)components
prop?Actual behavior
The Using MDX page (specifically the Components section and the MDX provider section) includes:
<video>
, writing an additional remark plugin for support of![]()
syntax can work)example.mdx
andexample.jsx
, not including the resulting HTMLexample.mdx
andexample.jsx
resulting HTML does not change with usage of the MDX providerRuntime
Node v20
Package manager
No response
OS
macOS
Build and bundle tools
Next.js
The text was updated successfully, but these errors were encountered: