Skip to content

Refactor or remove MDX on demand #2606

Open
@remcohaszing

Description

@remcohaszing

Initial checklist

Problem

I have been thinking about MDX on demand lately, and I see some quirks/problems

  1. The mdx-js/mdx package contains code for compiling code. But it also contains runtime code for evaluating the generated code. This seems like an odd place. For example, this adds runtime types for the compiler.
  2. It adds an extra option to the MDX compiler.
  3. The transform of a module to a function body runs after third party recma plugins. This adds complexity for recma plugins, which now have to deal with the possibility of a top-level return statement.
  4. The transform has bugs. For example:
    export * as foo from 'foo'
    export * from 'bar'
    transforms into:
    const _exportAll1 = await import(_resolveDynamicMdxSpecifier('foo'))
    const _exportAll2 = await import(_resolveDynamicMdxSpecifier('bar'))
    // …
    return {
      ..._exportAll1,
      ..._exportAll2
    }
    This should be:
    const _exportAll1 = await import(_resolveDynamicMdxSpecifier('foo'))
    const _exportAll2 = await import(_resolveDynamicMdxSpecifier('bar'))
    // …
    return {
      foo: _exportAll1,
      ..._exportAll2
    }
  5. Imports are resolved, but the JSX runtime is injected.
  6. Eval is evil. We do warn about the dangers of evaluating code, plenty I think. But still, I think we can discourage it further by moving this out of core.

Current solutions

The user can use run / evaluate. Alternatively they can use recma-module-to-function.

Proposed solutions

I have some solutions that we can implement partially or gradually.

  1. Base the current implementation on recma-module-to-function. I spotted some quirks for edge cases in recma-module-to-function which I’m still resolving, but those exist in the current MDX on demand implementation as well. Several of the now generated code for MDX on demand can be solved using a custom import implementation. This can be non-breaking. A naive untested implementation:
    function createImport(baseUrl, runtime) {
      _import.meta = {
        url: String(baseUrl),
        resolve
      }
    
      return _import
    
      function _import(specifier, options) {
        if (specifier === 'mdx:/jsx-runtime') {
          return runtime
        }
    
        return import(resolve(specifier), options)
      }
    
      function resolve(specifier) {
        if (typeof specifier !== "string") {
          return specifier
        }
    
        try {
          new URL(specifier)
          return specifier
        } catch {}
    
        if (
          specifier.startsWith("/") || 
          specifier.startsWith("./") ||
          specifier.startsWith("../")
        ) {
          return new URL(specifier, baseUrl).href
        }
    
        return specifier
      }
    }
  2. Move evaluate / run into a new package @mdx-js/on-demand. This means the core no longer needs the option outputFormat or any runtime code.
  3. Completely remove evaluate / run. We can still document an example like https://github.com/remcohaszing/recma-module-to-function#examples and keep the terminology MDX on demand to describe this pattern. Some (subjective) benefits:
    • Safer: This explicitly makes the user author the evaluation code using the JavaScript using the AsyncFunction constructor. I could even make it compatible with the Function constructor. This will trigger the ESLint rule no-new-func for many users, which I believe to be a good thing.
    • Greater flexibility: Import logic is very flexible and explicit. There’s no need to explain the concept of a base URL.
    • Simpler: There’s no special handling of JSX. It’s just an import now.
    • Teaching: I think this makes it more explicit that MDX gets compiled to regular JavaScript.
    • More compatibility: This approach works with anything that compiles to JavaScript. The same concept could be used to transform JavaScript, JSX, or TypeScript for example.

Option 3 is definitely a breaking change and would be semver major. Option 2 could be done earlier via a deprecated re-export, but IMO that’s not worth the effort until we do a semver major.

Metadata

Metadata

Assignees

No one assigned

    Labels

    🤞 phase/openPost is being triaged manually

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions