type TestFunction is a type predicate, not a generic predicate

[chmac]

Subject of the issue

I'm looking at this:

unist-util-is/index.d.ts

Lines 29 to 33 in 8c0a649

	type TestFunction<T extends Node> = (
	node: unknown,
	index?: number,
	parent?: Parent
	) => node is T

As I read the docs, and the code, the test is "converted" here:

unist-util-is/convert.js

Lines 18 to 20 in 8c0a649

	if (typeof test === 'function') {
	return test
	}

As I understand the docs I can have a function that tests the content of the node and returns boolean whether the node is a match for my conditions or not.

However, as I read the types, my function must be a type predicate that asserts whether or not my node is of a given TypeScript type.

I'm definitely not a TypeScript expert, so I may have gotten something wrong here. If so, apologies. If not, I can try to add an additional type test.

Your environment

v 4.0.1

Steps to reproduce

import { Node, Parent } from "unist";
import visit from "unist-util-visit";

export const insertBefore = (
  tree: Parent,
  predicate: (node: Node, index?: number, parent?: Parent) => boolean,
  transform: (node: Node) => Node
) => {
  visit(tree, predicate, (node, index, parent) => {});
};

Expected behaviour

I believe that my example above would run in javascript, but fails type checking.

Actual behaviour

No overload matches this call.
  Overload 1 of 2, '(tree: Node, test: string | any[] | Partial<Node> | TestFunction<Node>, visitor: Visitor<Node>, reverse?: boolean | undefined): void', gave the following error.
    Argument of type '(node: Node, index?: number | undefined, parent?: ParentNode | undefined) => boolean' is not assignable to parameter of type 'string | any[] | Partial<Node> | TestFunction<Node>'.
      Type '(node: Node, index?: number | undefined, parent?: ParentNode | undefined) => boolean' is not assignable to type 'TestFunction<Node>'.
        Types of parameters 'node' and 'node' are incompatible.
          Type 'unknown' is not assignable to type 'Node'.
  Overload 2 of 2, '(tree: Node, visitor: Visitor<Node>, reverse?: boolean | undefined): void', gave the following error.
    Argument of type '(node: Node, index?: number | undefined, parent?: ParentNode | undefined) => boolean' is not assignable to parameter of type 'Visitor<Node>'.
      Types of parameters 'node' and 'node' are incompatible.
        Type 'Node' is missing the following properties from type 'Node': baseURI, childNodes, firstChild, isConnected, and 47 more.  TS2769

[ChristianMurphy]

@chmac predicate is a narrower type than boolean, which is why TS is erroring in this case.
https://www.typescriptlang.org/docs/handbook/advanced-types.html#using-type-predicates

---

Using a TS Predicate ensures that when used with a conditional, inside the if scope, TypeScript knows that the variable passed, is the type defined by the generic in is.
For example:

unist-util-is/unist-util-is-test.ts

Lines 36 to 37 in 8c0a649

	const isHeading = (node: unknown): node is Heading =>
	typeof node === 'object' && node !== null && (node as Node).type === 'heading'

unist-util-is/unist-util-is-test.ts

Lines 72 to 76 in 8c0a649

	if (is<Heading>(heading, 'heading')) {
	const maybeHeading: Heading = heading
	// $ExpectError
	const maybeNotHeading: Element = heading
	}

coming into this function, TypeScript only knows heading is some variable, within the if scope, TypeScript knows that is must be type Node and more specifically a Heading node and allows safe access to all properties provided by that type.
Using a boolean would only safely allow narrowing the type to Node, because we'd have no way to determine what more specific type is wanted.
Allowing a fully generic type without a predicate would allow for a runtime exception such as the one in the example here: #16 (comment)

---

In your example try:

import { Node, Parent } from "unist";
import visit from "unist-util-visit";

export const insertBefore = (
  tree: Parent,
  predicate: (node: Node, index?: number, parent?: Parent) => node is Node,
  transform: (node: Node) => Node
) => {
  visit(tree, predicate, (node, index, parent) => {});
};

or even better

import { Node, Parent } from "unist";
import { Test } from "unist-util-is";
import visit from "unist-util-visit";

export const insertBefore = <T extends Node>(
  tree: Parent,
  predicate: Test<T>,
  transform: (node: T) => Node
) => {
  visit(tree, predicate, (node, index, parent) => {});
};

Also reference the type tests in https://github.com/syntax-tree/unist-util-visit/blob/master/types/unist-util-visit-tests.ts for more examples.

[chmac]

@ChristianMurphy I don't think I've 100% grok'd this yet, but will spend some more time on it. Thanks for the detailed explanation.

It seems like you made a reasoned decision which I misunderstood. What about adding something to the docs sharing how to use this package in TypeScript? I'd be happy to have a go at submitting a PR to that effect if that would be helpful.

[ChristianMurphy]

More documentation is welcome.
Rather than making it repository specific, it could be good to add it to a guide about using Unified with TypeScript. See unifiedjs/unifiedjs.github.io#7 for more information on guides.

Types like Test, Plugin, and Transformer are used in many packages, having a central guide could be helpful to point folks in the right direction.

[wooorm]

If I understand this conversation correctly, I believe I can close this.
If that is a mistake, please comment here and we can open this again.