Report use of MDX internals

[remcohaszing]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

MDX generates some internal variables in the compiled output. These variables are in scope and users could use them, but they they should not.

Solution

Report usage of the following identifiers:

• _components
• _createMdxContent

We should probably also report usage of the following variables:

• MDXLayout
• MDXContent

Alternatives

Consider all variables starting with an underscore to be private.

[wooorm]

We can report. Or crash. Or increment the name of our variables (_components1).

What about props? That works because of this.

[remcohaszing]

Or crash.

This makes me think you mistook this issue from one in the MDX repo. I’m ok with the compiler crashing, but not the language server.

Or increment the name of our variables (_components1).

IMO incrementing makes sense if the user declares a conflicting variable, not when they access it. In that case it still should be reported by the language server. Also this is more work IMO than it’s worth.

What about props?

props is explicitly allowed. wooorm/xdm#51

[wooorm]

This makes me think you mistook this issue from one in the MDX repo

Indeed. Because whether these variables should be accessed affects MDX as a whole? Something could be done here, or something could be done in @mdx-js/mdx

not when they access it

I meant when MDX defines it. MDX injects variable names. It chooses these names right now. It could also check the scope and choose different names.

props is explicitly allowed. wooorm/xdm#51

Yes. You did that. Because you thought it was a good idea for users to access internals / make it an API.
So, my question, what about props is good to access, and what about components/createMdxContent/MdxLayout/MdxContent is not good to access?

[ChristianMurphy]

Good idea @remcohaszing!
Reporting on certain identifiers feels a bit like a lint rule 🤔
I'm curious on folks thoughts whether that should live: here, in the remark lint, or in eslint mdx.

[remcohaszing]

props allow a user to pass content from one part of code they own (the invocation of the MDX component) to another part of the code they own (the implementation of the MDX component). I think this is almost as useful as JSX expressions in MDX in general. Also other APIs may inject props, i.e. a HOC or Next.js data fetching API. Or a documentation writer could use variables defined by a website developer.

_components feels more like an implementation detail. It’s a convenient intermediate variable to support props.components and useMdxComponents(). Also lower case components are added on an as-needed basis, so it’s shape would unreliable for direct use.

_createMdxContent is an implementation detail to support MDX layout and React providers.

_createMdxContent, MDXContent would yield infinite recursion if used as JSX components inside MDX. Perhaps this is only a problem if this is used from within MDXLayout or MDXContent, not if used from other ESM exports.

Actually using MDXLayout could be fine. There’s not really a problem with wrapping nested parts with the same component that by coincidence also wraps the MDX content.

[remcohaszing]

Reporting on certain identifiers feels a bit like a lint rule 🤔

Maybe, this is an interesting thought.

I was also thinking of filtering these names from autocompletion results.

[wooorm]

Also lower case components are added on an as-needed basis, so it’s shape would unreliable for direct use.

(side note: the distinction isn’t about lower-case vs upper-case btw)

Agree that it’s probably a bad idea to use _components.p but someone could be accessing _components.H1. A merge of passed and injected components.

An IIFE:

{
  (() => {
    // Access passed/injected component inside JS:
    return <_components.H1>Hi!</_components.H1>
  })()
}

If we want to allow this behavior, we should probably call it components and make sure it’s always defined.

MDXContent would yield infinite recursion

If used in the main body. MDX files can also export components. Which could reference the global scope. Someone could do something along the lines of

export function WrappedContent() {
  return <main><MDXContent /></main>
}

# Hi

[remcohaszing]

If we want to allow this behavior, we should probably call it components and make sure it’s always defined.

I get your point, but if we do so, this:

<HI />

is equivalent to:

<components.HI />

It would only be different if the MDX file imports or defines the HI component, at which point it would be better to pick a different name for the locally defined component that doesn’t conflict with injected components IMO.

Also someone could already define the components property on components:

<MDXContent components={{components: {HI}}} />

To introduce a new variable would be a breaking change.

[remcohaszing]

MDXContent would yield infinite recursion

If used in the main body. MDX files can also export components. Which could reference the global scope. Someone could do something along the lines of

export function WrappedContent() {
  return <main><MDXContent /></main>
}

# Hi

Yep, usage from mdxjsEsm nodes is fine.

[wooorm]

Yes, renaming to make things public or preventing their use would both be breaking.

[remcohaszing]

On the compiler level, yes. To report bad usage in the editor is not breaking.

[remcohaszing]

Another internal variable is _missingMdxReference. This may or may not be defined in development mode.