Add remark-directive-mdx to MDX extensions list

[re-xyr]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and discussions and couldn’t find anything or linked relevant results below
• I made sure the docs are up to date
• I included tests (or that’s not needed)

Description of changes

remark-directive-mdx is a remark plugin that transforms Markdown directives (:directive[]{}) parsed by remark-directive into MDX JSX elements. This change adds this plugin to the plugins list in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (0004605) to head (c2cc2dd).

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #2664   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         2649      2649           
  Branches         2         2           
=========================================
  Hits          2649      2649           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[remcohaszing]

Thanks! This project looks really cool. I do have some tips.

• I believe you don’t need to define _mdxExplicitJsx in any of the places where you do.
• Don’t use Object.assign() to modify nodes. Instead, replace them like this:

  visit(tree, ['containerDirective', 'leafDirective', 'textDirective'], (node, index, parent) => {
    if(index == null || parent == null) {
      return
    }
  
    // …
  
    parent.children[index] = newNode
  })

• Avoid type casting using as.
• I think you may get better help from TypeScript if you use object literals instead of unist-builder. I may be wrong though.
• You perform some logic to convert directive attributes, interpreted as HTML attributes, to JSX attributes. Your approach is a bit naive. This makes sense, as class → className is a well-known example, but there’s more. You may be interested to have a look at or use hast-util-properties-to-mdx-jsx-attributes. On the other hand, users write those attributes without HTML. So maybe you don’t want to perform any conversion at all.
• If you map over an array, it’s generally better to specify the generic type on the .map method or the return type of the mapper function. This provides better TypeScript support.
• In astroHandleLabel you generate a <Fragment /> element. However, you don’t insert a Fragment import.
• In your tests you build the AST manually and test on that. This way it’s fairly easy to make mistakes. I recommend testing your plugin with compile() and testing it against a string input as an actual user would use it.

[re-xyr]

Hi! Thanks for your detailed feedback.

I believe you don’t need to define _mdxExplicitJsx in any of the places where you do.

remarkMarkAndUnravel did this for all "explicit JSX elements", and it was called before external remark plugins. Maybe we can debate whether directives count as "explicit", or I missed something.

Don’t use Object.assign() to modify nodes. Instead, replace them [...]

From unist-util-visit:

Replacing node itself, if SKIP is not returned, still causes its descendants to be walked (which is a bug).

It would cause the old subtree to be traversed instead of the new one. Indeed, if I try to replace the node in the parent outright by parent!.children[index!] = newNode, the test suite fails:

[FAIL] test/index.test.ts > transforms directives to MDX JSX
AssertionError: expected { type: 'root', …(1) } to deeply equal { type: 'root', …(1) }

- Expected
+ Received

@@ -54,17 +54,13 @@
              },
            ],
            "type": "paragraph",
          },
          {
-           "attributes": [],
            "children": [],
-           "data": {
-             "_mdxExplicitJsx": true,
-           },
            "name": "inner-directive",
-           "type": "mdxJsxFlowElement",
+           "type": "leafDirective",
          },
        ],

the nested directive is not transformed.

Avoid type casting using as.

This is unavoidable; we are doing the same thing as remarkMarkAndUnravel, which "unravels" any <p> tag with only one MdxJsxTextElement in it into a standalone MdxJsxFlowElement. This causes the transformed MdxJsxFlowElement to contain PhrasingContent[], which requires a cast.

I think you may get better help from TypeScript if you use object literals instead of unist-builder. I may be wrong though.

In any case, one fewer dependency is generally a good idea. I removed the dependency on unist-builder.

You perform some logic to convert directive attributes, interpreted as HTML attributes, to JSX attributes. Your approach is a bit naive [...]

By default, the plugin performs no conversion on attributes; I just exposed a simple function that users can pass to the plugin to instruct it to do some basic transformations. If you think the way that function behaves is wrong or misleading, please elaborate and I'll mark the function as deprecated.

If you map over an array, it’s generally better to specify the generic type on the .map method or the return type of the mapper function. This provides better TypeScript support.

TypeScript inference seems to work fine to me, currently. Did you see any degenerate typing leaking out to the API?

In astroHandleLabel you generate a <Fragment /> element. However, you don’t insert a Fragment import.

This is already done by @astrojs/mdx.

But in general, my current API for handleLabel doesn't support non-local transformations. Maybe we can pass the parents stack to the user too?

In your tests you build the AST manually and test on that. This way it’s fairly easy to make mistakes. I recommend testing your plugin with compile() and testing it against a string input as an actual user would use it.

This sounds like a good idea. I will do it later.

[wooorm]

remarkMarkAndUnravel did this for all "explicit JSX elements", and it was called before external remark plugins. Maybe we can debate whether directives count as "explicit", or I missed something.

Depends what you want! If a user passes h1: MyComponent, I think they probably want to overwrite # this and not <h1>this</h1> or ::h1[this]

It would cause the old subtree to be traversed instead of the new one. Indeed, if I try to replace the node in the parent outright by parent!.children[index!] = newNode, the test suite fails:

Is this a good case for [SKIP, currentIndex]? Will the function be called again for the transformed node, ignore the old children, and then call for the new children?

By default, the plugin performs no conversion on attributes; I just exposed a simple function that users can pass to the plugin to instruct it to do some basic transformations. If you think the way that function behaves is wrong or misleading, please elaborate and I'll mark the function as deprecated.

Generally I think there should be no conversion for things that users write themselves, and that MDX already has options for conversion for the stuff that users do not write.

[re-xyr]

Depends what you want! If a user passes h1: MyComponent, I think they probably want to overwrite # this and not <h1>this</h1> or ::h1[this]

In that case, the plugin is already doing that (adding _mdxExplicitJsx: true to the elements it generates).

Is this a good case for [SKIP, currentIndex]? Will the function be called again for the transformed node, ignore the old children, and then call for the new children?

This works, thanks for the suggestion. It saves us some object clones.

Generally I think there should be no conversion for things that users write themselves, and that MDX already has options for conversion for the stuff that users do not write.

I think it would be nice to at least have the option to always use kebab-case for directive names; it works across all JSX frameworks (correct me if I'm wrong) and to me it makes the document more stylistically coherent. It only falls apart when you start using Web Components.

Transformations on attributes might be more objectionable since only React/Solid have a uniform convention for attributes/props casing. But considering remark-directive always transforms {.classname} to class="classname", I think that at least provides some justification for having some way to handle it for React.