syntax-tree/unist-util-visit

Type doesn't work when using array of tests

Closed this issue · 7 comments

Initial checklist

Affected packages and versions

unist-util-visit@4.1.0

Link to runnable example

https://codesandbox.io/s/react-markdown-debug-forked-sqvof

Steps to reproduce

  1. Open the codesandbox link
  2. Inspect the TypeScript error at line 25

Expected behavior

The visit method should be able to narrow the type of directiveVisitor and not throw an error.

Actual behavior

The visit method's type is not able narrow the type of directiveVisitor because it can't read the test ['textDirective', 'leafDirective', 'containerDirective']

Runtime

Node v14

Package manager

yarn v1

OS

macOS

Build and bundle tools

Next.js

TS doesn’t like arrays of things.
You can always get around it:

visit(tree, (node) => {
  if (node.type === "textDirective" || node.type === "leafDirective" || node.type === "containerDirective") {
    // things.
  }
})

@wooorm this still fails for what I'm trying to achieve since the node parameter isn't properly typed to include the properties name and attributes.

I don’t know what you’re trying to achieve? It’s apparently different from what you explained before or have in the codesandbox?

The node parameter is properly typed if you properly type tree

P.S.: I had a typo in the above code where search existed but shouldn’t have existed

Here is your CS changed to make it work: https://codesandbox.io/s/react-markdown-debug-forked-4iimg.
For more info on using types with remark-directive, see its readme: https://github.com/remarkjs/remark-directive#types

@wooorm The typo mislead me. I thought search was equal to ['textDirective', 'leafDirective', 'containerDirective'] and the visitor function was used as the third parameter.

Your solution does work. Thanks! However, ideally it would be great that the type would be able to properly narrow the visitor function. Although, I don't know if that's actually possible with TypeScript.

The problem is that TS sees an array (['a', 'b']) as string[], and then we can’t do anything with it anymore. We could try and add support for as const syntax:

    const search: Test = [
      "textDirective",
      "leafDirective",
      "containerDirective"
    ] as const
    visit(tree, search, (node) => {

(doesn’t work), but then again I don’t think this behavior is intuitive, I don’t think folks would think that they’d have to do that

I believe this is a typescript limitation that can’t be changed. I’m open to documenting it. Or adding as const support.