search

mdx-js / mdx

Markdown for the component era
https://mdxjs.com
MIT License
17.12k stars 1.14k forks source link

Docs: Missing explicit docs and workaround for overriding literal HTML elements in MDX #2503

Open karlhorky opened 4 days ago

karlhorky commented 4 days ago

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

  1. Set up the Next.js App Router + MDX app-dir-mdx example from Remco
  2. Remove experimental.mdxRs from next.config.js
  3. Add a line video: () => <div>video replacement</div> in mdx-components.tsx
  4. Add a line <video /> in app/message.mdx
  5. Observe <video /> appearing in HTML in DevTools, instead of the <div>
  6. Research and find the following information in docs and GitHub issues / PRs
    1. https://github.com/mdx-js/mdx/issues/821
    2. https://github.com/kentcdodds/mdx-bundler/issues/160
    3. https://github.com/mdx-js/mdx/pull/2050
    4. Workaround: https://github.com/mdx-js/mdx/pull/2052#issuecomment-1140519087
    5. Workaround: https://github.com/kentcdodds/mdx-bundler/issues/160#issuecomment-1140526121
    6. https://github.com/remcohaszing/rehype-mdx-import-media/issues/2#issuecomment-2198361734
  7. Attempt the workaround from 5iv. and 5v. (adding the rehypePlugin suggested by @wooorm)
  8. Observe <video /> still appearing in HTML in DevTools, instead of the <div>

Screenshot 2024-07-03 at 13 38 04

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 https://github.com/mdx-js/mdx/issues/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 use rehype-mdx-import-media with <video> out of the box without any additional plugins:

Expected behavior

The Using MDX page (specifically the Components section and the MDX provider section) should include:

  1. A new working workaround (documented as an officially-blessed configuration) for overriding / replacing HTML literal elements in MDX
  2. An explicit description of this intended behavior - that MDX will by default not override or replace any HTML literal elements (maybe with a link to the original issue as background?)
  3. A short code example showing what the components prop does (not only example.mdx and example.jsx, but also the resulting HTML)
    1. A short code example showing that the resulting HTML does not change with usage of the MDX provider
  4. A text description of what MDX does with "an object mapping component names to components", using preferred terminology:
    1. Verb "replaces" / "overrides" / alternative: is it preferred to use the word "replaces" or "overrides" like the community has been using? or something more like "renders" with a longer description?
    2. Plural noun "custom components" / "MDXComponents" / "MDX components" alternatives: what is the preferred terminology for describing the plural things that are being passed as the keys of the object passed to the components prop?

Actual behavior

The Using MDX page (specifically the Components section and the MDX provider section) includes:

  1. No current working workaround (although for <video>, writing an additional remark plugin for support of ![]() syntax can work)
  2. No explicit description that certain literal HTML elements will not be overridden
  3. Code example of only example.mdx and example.jsx, not including the resulting HTML
    1. A diff of example.mdx and example.jsx resulting HTML does not change with usage of the MDX provider
  4. No text description of what MDX does with "an object mapping component names to components", using preferred terminology

Runtime

Node v20

Package manager

No response

OS

macOS

Build and bundle tools

Next.js

wooorm commented 4 days ago

Heyyyyy!

This is intentional, because it isn’t needed, you can do:

Video: () => <div>video replacement</div>

And then:

<Video />

Why is this one character change, which is inspired by how JSX itself works (if you write <video> it’s plain, if you write <Video> it’s a reference), not viable for you?

karlhorky commented 10 hours ago

I haven't yet collected all of the reasons why others have said they wanted this, but I'll list some reasons why overriding <video> or other HTML literal tags may be desirable off the top of my head:

  1. copying and pasting HTML
  2. tooling integrations for HTML elements eg. linting for attributes which should be there
  3. greppability in codebase
  4. style preference towards usage of HTML elements instead of Markdown elements / JSX elements

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

4) A text description of what MDX does with "an object mapping component names to components", using preferred terminology

Screenshot 2024-07-07 at 18 32 15

-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 example.mdx and example.jsx

3) A short code example showing what the components prop does (not only example.mdx and example.jsx, but also the resulting HTML)

Screenshot 2024-07-07 at 18 48 42

Take this example:

```mdx path="example.mdx"
# Hello *<Planet />*

It can be imported from JavaScript and passed components like so:

// @filename: types.d.ts
import type {} from 'mdx'
// @filename: example.jsx
/// <reference lib="dom" />
/* @jsxImportSource react */
// ---cut---
import Example from './example.mdx' // Assumes an integration is used to compile MDX -> JS.

console.log(
  <Example
    components={{
      Planet() {
        return <span style={{color: 'tomato'}}>Pluto</span>
      }
    }}
  />
)

+This results in the following HTML: + +html +<h1>Hello <span style="color: tomato">Pluto</span></h1> +


Again, my example above may likely be missing something or needs tweaks to be 100% accurate for the wide use cases of MDX - it's intended as an illustration of what I mean, and something that would be a very helpful missing piece to showcase what `components` does.

## 2) [Using MDX -> Components section](https://mdxjs.com/docs/using-mdx/#components) - Explicit description that HTML literal elements not replaced

> 2) An explicit description of this intended behavior - that MDX will by default not override or replace any HTML literal elements (maybe with a link to [the original issue](https://github.com/mdx-js/mdx/issues/821) as background?)

![Screenshot 2024-07-07 at 19 01 00](https://github.com/mdx-js/mdx/assets/1935696/d86840e4-f7fd-4114-a728-678ada62585f)

```diff

-The following keys can be passed in `components`:
+The following keys can be passed in a `components` object:

-* HTML equivalents for the things you write with markdown such as `h1` for
-  `# heading` (see [§ Table of components][toc] for examples)
+* Markdown syntax-generated HTML elements eg. if you write `# heading` in your MDX file,
+   the key is `h1` (this does not include all HTML, see [§ Table of components][toc] for examples)
* `wrapper`, which defines the layout (but a local layout takes precedence)
-* `*` anything else that is a valid JavaScript identifier (`foo`,
+* anything else that is a valid JavaScript identifier (`foo`,
  `Quote`, `_`, `$x`, `a1`) for the things you write with JSX (like
  `<So />` or `<like.so />`, note that locally defined components take
  precedence)**‡**
+
+Caveat: the following keys in a `component` object will not be replaced:
+
+* HTML elements which do not use Markdown syntax eg. `<img>`, `<video>`, etc. in `.mdx` files
+  ([see background](https://github.com/mdx-js/mdx/issues/821)) - for these use cases, use a custom component like `Video: () => { ... }`

**‡** If you ever wondered what the rules are for whether a name in JSX (so `x`
in `<x>`) is a literal tag name (like `h1`) or not (like `Component`), they are
as follows:

A few first suggestions above.

Considering it more, I think that the potential confusion of HTML literal elements not being replaced would actually warrant another section showing:

To be a companion for the text to be 100% explicit that some components (such as literal HTML elements in the .mdx file) will not be replaced, and will have the original elements as in the .mdx file.

  • Why are the existing docs not clear? Emphasis to show what I mean

As in my diff above:

  • Isn’t that the example at “Here are some other examples of passing components”? It has text explaining each thing in the comments?

I think your comment was mostly referring to my 2) suggestion. It doesn't relate to 3) because 3 is about having an HTML result example directly next to the other example.mdx and example.jsx code blocks.

Including the screenshot of the example code block here for reference:

Screenshot 2024-07-07 at 19 17 50

I like this example code block, yes! I don't think that it describes in detail which things can and cannot be used (which I'm proposing in 2), and the comments are pretty easy to miss. But I love these types of examples, and I think it's very valuable.

3i) Using MDX -> MDX provider section - Resulting HTML of example.mdx and example.jsx

3i) A short code example showing that the resulting HTML does not change with usage of the MDX provider

Same idea as above with 3) - a separate HTML "result" code block. Showing that the resulting HTML is the same, to make the mental models concrete that the docs reader is building up and confirm their assumptions.

ChristianMurphy commented 55 minutes ago

Thanks for sharing @karlhorky!

This comes up often enough I'd be okay with there being some documentation. Similar to @wooorm I view overriding plain HTML as a really bad idea. So if it were included it may make sense to do something like Micromark extensions https://github.com/micromark/micromark?tab=readme-ov-file#extending-markdown

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.