Change types to base what visitor gets on tree

[wooorm]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)
• If applicable, I’ve added docs and tests

Description of changes

While the signature visitParents(tree, 'emphasis', (node: Emphasis) => void) already works (that is, assuming that the tree could potentially include an emphasis node, so not hast, and that the tree does not include an type: emphasis which is otherwise different from Emphasis), the other signatures were not as smart.

Given

const root: Root = {...}
visitParent(root, (node: ?) => {})

? is now filled with any node that is root or (recursively) in root.children.

EDIT: this now also figures out the signature with a test!

[wooorm]

Open to different names.
Previously I used SelfAndChildren.

We could also extract this type. Either in a repo: syntax-tree/typefest? Or in @types/unist.

[remcohaszing]

What about Descendant? Naming is hard.

[wooorm]

https://github.com/syntax-tree/unist#descendant

InclusiveDescendant to be sure?

[wooorm]

We could add type utilities for several things in Unist to types/unist perhaps? 🤔

[remcohaszing]

Maybe. Although really NodeInTree isn’t that bad either actually.

[remcohaszing]

I’m thinking it may be better to just use an existing AST for testing to ensure real world compatibility. Actually using @types/mdast here is probably even less effort.

[wooorm]

Yeah maybe. I thought it was wise to keep this small and contained

[wooorm]

Fixed the types. The “catch-all” overload is no longer needed.

Re #10 (comment):
It was needed because Needle (the node type to be found) could only be explicitly given (so through TypeScript), making the “catch-all” overload required for JavaScript.
I’ve now changed the behavior of the types to:

• find all node types in the tree
• extract node types that match the given test in the type system.

Take for example the docs in How to narrow Nodes in TypeScript, on lists:

if (is<List>(node, 'list')) {
  // If we’re here, node is List.
  //
  // `'list'` is compared to `node.type` to make sure they match.
  // `true` means a match, `false` means no match.
  //
  // `<List>` tells TypeScript to ensure `'list'` matches `List.type` and that
  // if `'list'` matches both `node.type` and `List.type`, we know that node is
  // `List` within this if condition.
}

(although this code is for Is<>, not for Visit<>).
The <List> is no longer needed explicitly in Visit, it is inferred.

[ChristianMurphy]

Nice!
Is visiting a generic Node still supported? Or going forward will be always need to start with a more specific tree/node type?

[wooorm]

If you mean that visit<SomeNodeType>(anObjectThatLooksLikeANode, 'some-node-type', (node: SomeNodeType) => {}) still works, then yes.
Because that “replaces” Tree from anObjectThatLooksLikeANode with SomeNodeType instead, and assuming that 'some-node-type' matches it, will pass that node type as node.

There are some complexities with for example visit({type: 'heading', depth: 1, children: []}, 'heading') erroring, because depth and children do not exist on Node.
Or visit(typedTree, {type: 'heading'}, (node: Heading) => {}) erroring, because TS {type: 'heading'} is turned into {type: string} which then yields all nodes instead.

[ChristianMurphy]

it may be good to have one or two type tests showing it works with plain Node/Parent #10 (comment)

[wooorm]

Added some tests for implicit trees.

Unfortunately I was wrong. It doesn’t work :'(

visit<SomeNodeType>(anObjectThatLooksLikeANode, 'some-node-type', (node: SomeNodeType) => {})

Does not work, because anObjectThatLooksLikeANode is probably not assignable to SomeNodeType

[wooorm]

@ChristianMurphy Still good to go? I think this should be a major?

[ChristianMurphy]

Looks good! 👍
And yes, with some potentially breaking cases around generic Node, semver major sounds more appropriate.