Type error when using `strip-markdown` in `unified`

TomerAberbach commented 5 months ago

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)

Affected packages and versions

6.0.0

Link to runnable example

No response

Steps to reproduce

node v21.7.3

unified()
  // ...
  .use(stripMarkdown)
  // ...

Expected behavior

No type error

Actual behavior

No overload matches this call.
  Overload 1 of 3, '(preset?: Preset | null | undefined): Processor<Root, undefined, undefined, undefined, undefined>', gave the following error.
    Type '(options?: Readonly<Options> | null | undefined) => (tree: Root) => Root' has no properties in common with type 'Preset'.
  Overload 2 of 3, '(list: PluggableList): Processor<Root, undefined, undefined, undefined, undefined>', gave the following error.
    Argument of type '(options?: Readonly<Options> | null | undefined) => (tree: Root) => Root' is not assignable to parameter of type 'PluggableList'.
  Overload 3 of 3, '(plugin: Plugin<[], Root, Root>, ...parameters: [] | [boolean]): Processor<Root, undefined, undefined, Root, Root>', gave the following error.
    Argument of type '(options?: Readonly<Options> | null | undefined) => (tree: Root) => Root' is not assignable to parameter of type 'Plugin<[], Root, Root>'.
      Type '(tree: Root) => Root' is not assignable to type 'void'.ts(2769)

It seems like the .use(...) method wants the plugin to return nothing (void). Passing stripMarkdown as () => void fixes the type error

Runtime

Other (please specify in steps to reproduce)

Package manager

pnpm

OS

macOS

Build and bundle tools

No response

ChristianMurphy commented 5 months ago

Welcome @TomerAberbach! 👋 Sorry you ran into a spot of trouble.

Are you running the latest versions of unified and this plugin together? Usually an error like that comes from using an outdated major version of one or the other.

github-actions[bot] commented 5 months ago

Hi! Thanks for taking the time to contribute! This has been marked by a maintainer as needing more info. It’s not clear yet whether this is an issue. Here are a couple tips:

Spend time framing the issue! The more time you put into it, the more we will
Often, maintainers respond with why for several back and forths; rubber duck debugging might help avoid that
Folks posting issues sometimes fall for xy problems: asking for a certain solution instead of raising the root problem

Thanks, — bb

TomerAberbach commented 5 months ago

Hmmm, looks like I'm using unified@11.0.4. I think that's the latest?

wooorm commented 5 months ago

Can you trash your node_modules/ and package-locks—and if you use non-npm tools, similar caches—and npm install again? This should not happen. There’s probably something outdated in your dependencies.

TomerAberbach commented 5 months ago

Just tried, but didn't seem to make a different sadly.

What I see for strip-markdown:

/**
 * Remove markdown formatting.
 *
 * * remove `code`, `html`, `horizontalRule`, `table`, `toml`, `yaml`, and
 *   their content
 * * render everything else as simple paragraphs without formatting
 * * uses `alt` text for images
 *
 * @param {Readonly<Options> | null | undefined} [options]
 *   Configuration (optional).
 * @returns
 *   Transform.
 */
export default function stripMarkdown(options?: Readonly<Options> | null | undefined): (tree: Root) => Root;
export type Heading = import('mdast').Heading;
export type Image = import('mdast').Image;
export type ImageReference = import('mdast').ImageReference;
export type InlineCode = import('mdast').InlineCode;
export type Nodes = import('mdast').Nodes;
export type Paragraph = import('mdast').Paragraph;
export type Parents = import('mdast').Parents;
export type Root = import('mdast').Root;
export type RootContent = import('mdast').RootContent;
export type Text = import('mdast').Text;
/**
 * Transform a node.
 */
export type Handler = (node: any) => Array<Nodes> | Nodes | null | undefined;
/**
 * Handlers.
 */
export type Handlers = Partial<Record<Nodes['type'], Handler>>;
/**
 * Configuration.
 */
export type Options = {
    /**
     * List of node types to leave unchanged (optional).
     */
    keep?: ReadonlyArray<Nodes['type']> | null | undefined;
    /**
     * List of node types to remove (or replace, with handlers) (optional).
     */
    remove?: ReadonlyArray<readonly [Nodes['type'], Handler] | Nodes['type']> | null | undefined;
};

What I see for unified:

  /**
   * Configure the processor to use a plugin, a list of usable values, or a
   * preset.
   *
   * If the processor is already using a plugin, the previous plugin
   * configuration is changed based on the options that are passed in.
   * In other words, the plugin is not added a second time.
   *
   * > 👉 **Note**: `use` cannot be called on *frozen* processors.
   * > Call the processor first to create a new unfrozen processor.
   *
   * @example
   *   There are many ways to pass plugins to `.use()`.
   *   This example gives an overview:
   *
   *   ```js
   *   import {unified} from 'unified'
   *
   *   unified()
   *     // Plugin with options:
   *     .use(pluginA, {x: true, y: true})
   *     // Passing the same plugin again merges configuration (to `{x: true, y: false, z: true}`):
   *     .use(pluginA, {y: false, z: true})
   *     // Plugins:
   *     .use([pluginB, pluginC])
   *     // Two plugins, the second with options:
   *     .use([pluginD, [pluginE, {}]])
   *     // Preset with plugins and settings:
   *     .use({plugins: [pluginF, [pluginG, {}]], settings: {position: false}})
   *     // Settings only:
   *     .use({settings: {position: false}})
   *   ```
   *
   * @template {Array<unknown>} [Parameters=[]]
   * @template {Node | string | undefined} [Input=undefined]
   * @template [Output=Input]
   *
   * @overload
   * @param {Preset | null | undefined} [preset]
   * @returns {Processor<ParseTree, HeadTree, TailTree, CompileTree, CompileResult>}
   *
   * @overload
   * @param {PluggableList} list
   * @returns {Processor<ParseTree, HeadTree, TailTree, CompileTree, CompileResult>}
   *
   * @overload
   * @param {Plugin<Parameters, Input, Output>} plugin
   * @param {...(Parameters | [boolean])} parameters
   * @returns {UsePlugin<ParseTree, HeadTree, TailTree, CompileTree, CompileResult, Input, Output>}
   *
   * @param {PluggableList | Plugin | Preset | null | undefined} value
   *   Usable value.
   * @param {...unknown} parameters
   *   Parameters, when a plugin is given as a usable value.
   * @returns {Processor<ParseTree, HeadTree, TailTree, CompileTree, CompileResult>}
   *   Current processor.
   */
  use(value, ...parameters) {
    // ...
  }

// ...

/**
 * @template {Array<unknown>} [PluginParameters=[]]
 *   Arguments passed to the plugin (default: `[]`, the empty tuple).
 * @template {Node | string | undefined} [Input=Node]
 *   Value that is expected as input (default: `Node`).
 *
 *   *   If the plugin returns a {@link Transformer `Transformer`}, this
 *       should be the node it expects.
 *   *   If the plugin sets a {@link Parser `Parser`}, this should be
 *       `string`.
 *   *   If the plugin sets a {@link Compiler `Compiler`}, this should be the
 *       node it expects.
 * @template [Output=Input]
 *   Value that is yielded as output (default: `Input`).
 *
 *   *   If the plugin returns a {@link Transformer `Transformer`}, this
 *       should be the node that that yields.
 *   *   If the plugin sets a {@link Parser `Parser`}, this should be the
 *       node that it yields.
 *   *   If the plugin sets a {@link Compiler `Compiler`}, this should be
 *       result it yields.
 * @typedef {(
 *   (this: Processor, ...parameters: PluginParameters) =>
 *     Input extends string ? // Parser.
 *        Output extends Node | undefined ? undefined | void : never :
 *     Output extends CompileResults ? // Compiler.
 *        Input extends Node | undefined ? undefined | void : never :
 *     Transformer<
 *       Input extends Node ? Input : Node,
 *       Output extends Node ? Output : Node
 *     > | undefined | void
 * )} Plugin
 *   Single plugin.
 *
 *   Plugins configure the processors they are applied on in the following
 *   ways:
 *
 *   *   they change the processor, such as the parser, the compiler, or by
 *       configuring data
 *   *   they specify how to handle trees and files
 *
 *   In practice, they are functions that can receive options and configure the
 *   processor (`this`).
 *
 *   > 👉 **Note**: plugins are called when the processor is *frozen*, not when
 *   > they are applied.
 */

wooorm commented 5 months ago

And all of that looks fine. It probably has more to do with the code you have. The things you have installed, as this repo itself also uses unified. It’s tested. It works. Perhaps an outdated @types/mdast? You can find more by running npm ls @types/mdast (and npm why @types/mdast).

wooorm commented 5 months ago

pnpm

I would assume that this is the problem. Clear your pnpm caches. Update your pnpm caches. That probably improves things

TomerAberbach commented 5 months ago

Okayyyyy so I dug into this a bunch and it seems the culprit was me doing this somewhere in my code:

declare module 'unified' {
  interface CompileResultMap {
    mdNode: MdNode
  }
}

I did this because there's some (other) place in my code where I want to return the mdast node as the final output. Not sure if this is the best way to do it, but along with adding to CompileResultMap above, I also then did something like this in the code:

unified()
      .use(...)
      .use(...)
      .use(...)
      .use(...)
      .use(function () {
        this.compiler = mdAst => mdAst
      })
      .process(markdown)

Now for whatever reason, declaring that messes up the overloads of .use(...) (everywhere else in the code) such that it no longer likes it when you give an md-node-returning plugin that returns non-void. It actually seems like if you do this:

declare module 'unified' {
  interface CompileResultMap {
    htmlNode: HtmlNode
  }
}

Then it messes up HTML plugins in the same way (e.g. remark-rehype has a similar type error when you do this because it returns Promise<undefined>)

So I guess there's some incompatibility with specifying a node type as a compile result and using plugins that output that node type?

Anyway, a bit out of my depths here at this point... Should I be getting a plain node out of the unified() pipeline in a better way than what I described above? If not, do you think this is a real bug here? (the issue with CompileResultMap?)

Thanks in advance :)

wooorm commented 5 months ago

Ohh, yeah, no that’s pretty wrong on different accounts tho the code here is minimal. I think that what you are looking for is parse and run on unified. Not process. See the diagram at https://github.com/unifiedjs/unified#overview.

github-actions[bot] commented 5 months ago

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

remarkjs / strip-markdown