Support `Node` typeParam in `Pluggable` and `PluggableList`

[remcohaszing]

Initial checklist

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

Problem

Currently the type of the AST is any in the Pluggable and PluggableList types. I think it would be nice if they can be specified explicitly, and default unist Node.

For example, this is a nice to have

import { compile } from '@mdx-js/mdx'

compile('', {
  remarkPlugins: [
    () => (ast) => {
      // ast is of type mdast.Root
    }
  ],
  rehypePlugins: [
    () => (ast) => {
      // ast is of type hast.Root
    },
    // This is a type error. It belongs in remarkPlugins
    remarkFrontmatter
  ]
})

Solution

In https://github.com/unifiedjs/unified/blob/main/index.d.ts#L592-L606:

  /**
   * A union of the different ways to add plugins and settings.
   *
   * @typeParam PluginParameters
   *   Plugin settings.
   */
  export type Pluggable<PluginParameters extends any[] = any[], Root = Node> =
    | PluginTuple<PluginParameters, Root, Root>
    | Plugin<PluginParameters, Root, Root>
    | Preset

  /**
   * A list of plugins and presets.
   */
  - export type PluggableList = Pluggable[]
  + export type PluggableList<Root = Node> = Pluggable<any[], Root>[]

Alternatives

Keep it as-is.

[wooorm]

Sure, probably good to do? Don’t see big downsides?

PluggableList is complex because it could be used for remark plugins, then remark-rehype, and then more rehype plugins: it doesn’t have to be one particular AST

[ChristianMurphy]

Improvements are welcome! This is something which would be good to smoke test downstream on some remark and rehype plugins to make sure they're still compatible.

I'd also note that it would be even more ideal, if we could make the AST type for plugins variadic. So that it can follow the chain of plugins and make sure they are combined together correct. And/Or to make sure that the plugin options line up with what the plugin actually accepts.

There was some exploration of this in: https://github.com/unifiedjs/unified/pull/92 https://github.com/unifiedjs/unified/pull/96 which ran into limitations of TypeScript at the time, and weren't merged, but several TypeScript major versions later, it may be possible now.

[wooorm]

So that it can follow the chain of plugins and make sure they are combined together correct.

Would that mean sort of similar to how a processor has an InputTree, CurrentTree, OutputResult? So like, PluggableList would have InputTree and OutputTree, where InputTree is the tree of the first transform and OutputTree the output of the last transform?

---

Aside indeed: this might run into TS limitations. If we’re going to do something, I’d prefer moving to JSDoc if that’s now possible (and/or maybe clean the JS API a bit if that makes that possible?) over doing more and more complex types

[ChristianMurphy]

Would that mean sort of similar to how a processor has an InputTree, CurrentTree, OutputResult? So like, PluggableList would have InputTree and OutputTree, where InputTree is the tree of the first transform and OutputTree the output of the last transform?

Exactly

Aside indeed: this might run into TS limitations. If we’re going to do something, I’d prefer moving to JSDoc if that’s now possible (and/or maybe clean the JS API a bit if that makes that possible?) over doing more and more complex types

👍

[remcohaszing]

I think this is still not possible indeed due to TypeScript limitations. Still, it would be useful for MDX plugin arrays, where the AST type isn’t supposed to change.

Alternatively the MDX plugin types could be changed to Array<Plugin<any, mdast.Root, mdast.Root>>, Array<Plugin<any, hast.Root, hast.Root>>, and Array<Plugin<any, estree.Program, estree.Program>> respectively.

[wooorm]

That alternative seems pretty nice (though: presets?). Why not do that?

[ChristianMurphy]

It may be possible for a limited number of plugins: https://github.com/unifiedjs/unified/pull/92 has an example which, I think, has some relevance to what you propose. But it doesn't scale beyond a fixed number of plugins.

Alternatively the MDX plugin types could be changed to Array<Plugin<any, mdast.Root, mdast.Root>>, Array<Plugin<any, hast.Root, hast.Root>>, and Array<Plugin<any, estree.Program, estree.Program>> respectively.

This could also work

[wooorm]

MDX plugin arrays

Presets and nested plugin lists could also exist there btw.

[wooorm]

After once again spending days trying to improve the types somewhat, I’d say: try it out, but I don’t have high hopes. It’s just all quite complex...

[wooorm]

Closing as I have strong doubts this issue is currently actionable, due to TS limitations around complex types. Improvements to types are of course welcome.

[github-actions[bot]]

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.

[github-actions[bot]]

Hi team! I don’t know what’s up as there’s no phase label. Please add one so I know where it’s at.

Thanks, — bb