Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Types not regonized on nodes #37

Closed
4 tasks done
kam-st opened this issue May 2, 2024 · 23 comments
Closed
4 tasks done

Types not regonized on nodes #37

kam-st opened this issue May 2, 2024 · 23 comments
Labels
🙋 no/question This does not need any changes 👎 phase/no Post cannot or will not be acted on

Comments

@kam-st
Copy link

kam-st commented May 2, 2024

Initial checklist

Affected packages and versions

unist-util-visit

Link to runnable example

No response

Steps to reproduce

In following code cannot find types for tagName, properties and children.

const result = await unified()
    .use(remarkParse, { fragment: true })
    .use(remarkRehype)
    .use(rehypeSlug)
    .use(() => {
      return (tree) => {
        console.log(tree);
        visit(tree, 'element', (node) => {
          // @ts-ignore
          if (node.tagName == 'h2') {
            // console.log(node);
            // @ts-ignore
            const id = node.properties.id;
            // @ts-ignore
            const heading = node.children[0].value;
            toc.push({ id, title: heading });
          }
        });
      };
    })
    .use(rehypeStringify)
    .process(fileContent);

  return toc;
}

Expected behavior

Types should be recognized.

Actual behavior

node is of type 'never'

Affected runtime and version

node@20 lts

Affected package manager and version

No response

Affected OS and version

No response

Build and bundle tools

No response

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels May 2, 2024
@wooorm
Copy link
Member

wooorm commented May 2, 2024

Type your plugin. See all plugins for how to do it.

@wooorm wooorm closed this as not planned Won't fix, can't repro, duplicate, stale May 2, 2024

This comment has been minimized.

@wooorm wooorm added the 🙋 no/question This does not need any changes label May 2, 2024
Copy link

github-actions bot commented May 2, 2024

Hi! Thanks for reaching out! Because we treat issues as our backlog, we close issues that are questions since they don’t represent a task to be completed.

See our support docs for how and where to ask questions.

Thanks,
— bb

@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels May 2, 2024
@ChristianMurphy
Copy link
Member

Adding on, there is a guide on this very topic you can reference @kam-st https://unifiedjs.com/learn/recipe/tree-traversal-typescript/

@omonk
Copy link

omonk commented Jun 21, 2024

I'm doing something along the same lines but the typings aren't working for me when I'm using Root from hast

import remarkParse from 'remark-parse';
import remarkRehype from 'remark-rehype';
import { unified } from 'unified';
import { visit } from 'unist-util-visit';
import { is } from 'unist-util-is';
import type { Root  } from 'hast';

unified()
    .use(remarkParse)
    .use(remarkRehype)
    .use(() => (hast: Root) => {
      visit(hast, 'element', (node) => {
        const isAnchor = is(node, 'anchor');

        if (isAnchor) {
          const href = node.properties.href; 
        }
      });
    })

The visitor node type is Element, but selecting node.properties results in a TS error

Property 'properties' does not exist on type 'never'.
  The intersection 'Element & Node & { type: "anchor"; }' was reduced to 'never' because property 'type' has conflicting types in some constituents.

The node from remarkRehype will be a Root hast will it not?

@wooorm
Copy link
Member

wooorm commented Jun 21, 2024

The explanation is sometimes the simplest possible explanation: You get a type error because you have a type error. Your code has an error.

'anchor' is not an existing node type in hast (that is what you check by using unist-util-is).
'anchor' is also not an element name known to HTML (that is what you would check by using hast-util-is-element).

No clue what you are doing. Place some console.logs in your code to see if things work.

@omonk
Copy link

omonk commented Jun 21, 2024

Thank you for such a speedy reply! I was probably just getting a bit confused with all the different types exported. This works without any TS issues now.

import { type Root } from 'hast';
import { isElement } from 'hast-util-is-element'; 
import { hasProperty } from 'hast-util-has-property';

// 

.use(() => (hast: Root) => {
  visit(hast, 'element', (node) => {
    const isAnchor = isElement(node, 'a');
    const hasHref = hasProperty(node, 'href');

    if (isAnchor && hasHref) {
      const href = node.properties.href;
      const parsed = path.parse(href as string);

      if (parsed.root === '/') {
        classnames(node, ['js-hook__internal-md-link']);
      }
    }
  });
})

@wooorm
Copy link
Member

wooorm commented Jun 21, 2024

you might not need those utilities:

-   const isAnchor = isElement(node, 'a');
-   const hasHref = hasProperty(node, 'href');
-
-   if (isAnchor && hasHref) {
+   if (node.tagName === 'a' && node.properties.href) {

p.s. thanks for sponsoring me! :)

@omonk
Copy link

omonk commented Jun 21, 2024

Maybe not but it makes me look like I know what I'm doing to the rest of my colleagues.

My pleasure, thank you for all of your work 🙏

@karlhorky
Copy link

karlhorky commented Jul 21, 2024

Type your plugin. See all plugins for how to do it.

  1. What does the plugin typing have to do with it? Shouldn't visit() already return a non-any / non-never type for node, given some valid string?

Eg. given the DX of other TypeScript- or JSDoc-supporting libraries, I would expect with a simple call to visit(), node would already be typed:

import {Node} from 'unist';
import { visit } from 'unist-util-visit';
import '@types/unist';

visit({} as Node, 'element', (node) => {
                           // ^? never
});

Playground

Edit: it seems like unist.Node may be the incorrect type for the code above.

I was using that because it's the type that is set when using import('unified').Plugin as such:

import {Plugin} from 'unified'

const plugin: Plugin = () => {
  return (tree) => {
        // ^? unist.Node
  }
}

Playground

But maybe unified.Plugin shouldn't be used to type remark plugins... 🤔 Or maybe only with special type arguments


  1. Also, it would be awesome for type discoverability if the check supported IntelliSense suggestions:

Screenshot 2024-07-21 at 18 15 09

Happy to open new separate issues for either of these 2 points, if they are accepted as possible improvements.

@karlhorky
Copy link

karlhorky commented Jul 21, 2024

Type your plugin. See all plugins for how to do it.

Also, separately, related to plugin typing:

Is there an easy-to-find example of how to get proper types without any or never in some remark plugin?

Looking through the remark List of Plugins, I found many which do not show typing related to the node in the original example:

Doing something like this doesn't change it from the never type:

import {Plugin} from 'unified'
import {Node} from 'unist'
import { visit } from 'unist-util-visit';
import '@types/unist'

function plugin(): ReturnType<Plugin> {
  return (tree: Node) => {
    visit(tree, 'element', (node) => {
                         // ^? never
    })
  }
}

Playground

@karlhorky
Copy link

Hmm... after digging for a while, it seems like remark-code-title may have working types for this:

import { visit } from "unist-util-visit";
import type * as mdast from "mdast";
import type * as unified from "unified";

export const remarkCodeTitle: unified.Plugin<[], mdast.Root> = () => {
  return (tree, file) => {
    visit(tree, "code", (node, index, parent) => {
                       // ^? mdast.Code
    })
  }
}

Playground

Not sure if this is the right thing to do, but this looks like the closest so far... 🤔

@karlhorky
Copy link

karlhorky commented Jul 21, 2024

For anyone else coming here while trying to build a remark plugin:

Seems that the key for a remark plugin is this mdast.Root type, which needs to be manually set.

And then an element type that would work here would be eg. 'image', which will return the mdast.Image type:

import {Root} from 'mdast';
import { visit } from 'unist-util-visit';

visit({} as Root, 'image', (node) => {
                         // ^? mdast.Image
});

Playground

To follow the recommendation to type the plugin, then it would look like the remark-code-title example:

import { visit } from "unist-util-visit";
import type { Root } from "mdast";
import type { Plugin } from "unified";

export const remarkCodeTitle: Plugin<[], Root> = () => {
  return (tree, file) => {
    visit(tree, "image", (node, index, parent) => {
                       // ^? mdast.Image
    })
  }
}

Playground


Edit: The behavior below is a bug in the TypeScript Playground

With JSDoc, it doesn't appear to be easy to get a typed node out without multiple manual annotations 🤔 :

import { visit } from "unist-util-visit";

// Only for TS Playground demo to download modules
import 'unified';
import 'mdast';

/** @type {import('unified').Plugin<[], import('mdast').Root>} */
export const remarkCodeTitle = () => {
  /**
   * @param {import('mdast').Root} tree
   */
  return (tree, file) => {
    visit(tree, "image",
      /**
       * @param {import('mdast').Image} node
       */
      (node, index, parent) => {
      // ^? mdast.Image
      }
    )
  }
}

Playground

Trying to avoid the multiple manual annotations, the type of node is any:

import { visit } from "unist-util-visit";

// Only for demo
import 'unified';
import 'mdast';

/** @type {import('unified').Plugin<[], import('mdast').Root>} */
export const remarkCodeTitle = () => {
  return (tree, file) => {
    visit(tree, "image",
      (node, index, parent) => {
      // ^? any
      }
    )
  }
}

Playground

@wooorm
Copy link
Member

wooorm commented Jul 22, 2024

Type the input. This issue is posted at a utility. So how to type things depends. If you are making plugins as you are doing here, then it means typing the plugins. You do not need Plugin. Adding types to things, in this case parameters, is fine:

/**
 * @import {Root} from 'mdast'
 */

import {visit} from 'unist-util-visit'

export function myPlugin() {
  /**
   * @param {Root} tree
   * @returns {undefined}
   */
  return function (tree) {
    visit(tree, 'image', function (node) {
      // `node` is typed.
    })
  }
}

@wooorm
Copy link
Member

wooorm commented Jul 22, 2024

I would not recommend looking at one particular userland example and base all your thinking on it. Instead, look at several examples, particularly the ones maintained by us. For remark, look in the remark organization: https://github.com/orgs/remarkjs/repositories?type=source.

@karlhorky
Copy link

karlhorky commented Jul 22, 2024

Type the input. This issue is posted at a utility. So how to type things depends. If you are making plugins as you are doing here, then it means typing the plugins. You do not need Plugin. Adding types to things, in this case parameters, is fine:

Edit: The behavior below is a bug in the TypeScript Playground

That would be a bit better, but it doesn't seem to yield a non-any type with JSDoc:

/**
 * @import {Root} from 'mdast'
 */

import { visit } from "unist-util-visit";

// Only for demo
import 'unified';
import 'mdast';

export function myPlugin() {
  /**
   * @param {Root} tree
   * @returns {undefined}
   */
  return function (tree) {
    visit(tree, 'image', function (node) {
                                // ^? any
    })
  }
}

Playground

I can open a separate issue for this if this is considered a bug in unist-util-visit.

@wooorm
Copy link
Member

wooorm commented Jul 22, 2024

I do not know why your playground doesn’t work. Perhaps your config isn‘t set up to support JavaScript, or Node, or perhaps packages don’t load.
It works locally, and that is how I have typed 100s of projects

@remcohaszing
Copy link
Member

remcohaszing commented Jul 22, 2024

For some reason the playground is unable to load the unist types. As a result, the type of node is not any. It is unresolved any (see microsoft/TypeScript#58960). This is why node is not properly resolved on the playground. The unist-util-visit types are working correctly. Please report an issue with the TypeScript playground.

playground

@karlhorky
Copy link

karlhorky commented Jul 22, 2024

I do not know why your playground doesn’t work.

It works locally

Indeed, in my reproduction repo I can confirm that the type is resolved to mdast.Image locally:

CI is also running through - no TypeScript errors or typescript-eslint problems reported: https://github.com/karlhorky/repro-unist-util-visit-node-any/actions/runs/10038028342/job/27739090729

Interesting, so the TS Playground has a bug somewhere here... 🤔

@wooorm
Copy link
Member

wooorm commented Jul 22, 2024

I would guess that it tries to load the mdast npm package instead of @types/mdast.

@karlhorky
Copy link

karlhorky commented Jul 22, 2024

Not sure that's related - I think the import 'mdast'; line is working to load @types/mdast, hovering over Root in the playground shows the type information:

Maybe Remco's comment indicates an unresolved any somewhere in the types...? I can't observe this anywhere in local IDE environment though.

@remcohaszing
Copy link
Member

@types/mdast is resolved fine in the playground, @types/unist is not. Hence there’t an unresolved any in the playground, but not locally. This can be observed in the network requests made as well. This issue is not related to our types.

👇

Please report an issue with the TypeScript playground.

@karlhorky
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 no/question This does not need any changes 👎 phase/no Post cannot or will not be acted on
Development

No branches or pull requests

6 participants