If a document has no activeElement then it cannot have a selection. Furthermore, isTextField and isNumberInput access the passed in element completely unguarded, so the assumption here was always that activeElement was defined.

range is not a boolean so we cast it to a boolean to appease the return value.

nonTextInputs is a string[] and because element.type could be undefined | string it doesn't fit in the box of a string[]. Checking for it's presence first turns it into a string which we can then pass to the includes function on the next line.

Pretty new to this. Why is Partial needed?

Partial is needed because the Element itself could be any HTML element that we're testing to check to see if it's an input, textarea or contentEditable Element. I wasn't sure how else to express that the Input part is optional and I think this is what @sirreal suggested as a potential solution to this problem (though I may be mis remembering, I can't find the original comment where that was suggested.

I believe this was introduced because we have some type that HTMLInputElement extends, but we don't know that it's an HTMLInputElement.
Partial<HTMLInputElement> doesn't seem ideal to me, it's typically used when we have an interface with optional properties. This usage is really that we have an Element (I believe, could be another type) and want to use it safely if it satisfies HTMLInputElement.

In my opinion, a better approach would be to accept the element type we expect to pass in (Element?), and use a type guard to narrow our element to our desired type. I implemented a type guard like this in #29220 to handle this type of case:

@sarayourfriend I believe you're thinking of this comment: #30030 (comment)

Right, maybe just HTMLElement? We should allow any element to be checked with this function.

Aha, yes, I did try to create a type guard but got stuck on contentEditable... I'll revisit this though and see if I can make it work with a type guard.

Meaning: sometimes the flexibility is nice so you don't have to check the type before calling a function that's being strict about the type. I think this is generally good for all functions in this package.

Looks like I just needed a couple type casts and it works out well even without the type guard.

HTMLElement doesn't work?

It seems like HTMLElement is a narrow type than is necessary. From the documentation on Element:

Element is the most general base class from which all objects in a Document inherit. It only has methods and properties common to all kinds of elements. More specific classes inherit from Element

Whereas HTMLElement is:

Any HTML element. Some elements directly implement this interface, while others implement it via an interface that inherits it.

Maybe it should be left as HTMLElement instead? 🤔 I'm not sure 😬 I'll defer to @sirreal again on this one, sorry to pull you back in Jon!

AS far as I know, Element can also be SVGElement and perhaps custom types. Just Element sounds good though. 🤷‍♀️

I'd say it's good practice to generalize parameter types as much as possible. That will limit what we can do with the values internally. Externally, it will allow these functions to be used in more places.

For example, this function checks the nodeName property, which exists on Node (from which Element inherits):

export default function isHTMLInputElement( node: Node ): node is HTMLInputElement {
	return node.nodeName === 'INPUT';
}

That seems ideal to me. The parameter is exactly as strict as it must be for it to be used internally.

Why is it disabled here and enabled below?

Because the element is HTMLInputElement syntax isn't considered a "valid type" by the JSDoc parser (doctrine) that the eslint JSDoc rules use. If you search the codebase you'll see that we've unfortunately had to disable these all over the place.

Are we planning to pass null or undefined? I see in isNumberInput we already require an Element, so can't we require it here?

If this function is used in other places it might be helpful to also handle null and undefined correctly. On the other hand, might be a YAGNI situation. I'll leave it up to you.

It's easy to widen types at any point, but a breaking change to narrow them. If this is ever released accepting X | null | undefined , that's more difficult to narrow to X than the other way around. A function that accepts X can start the wider accepting X | null | undefined at any point.

I agree, it might be handy to accept null for taking input from things like Document.querySelector or Node.parentNode that may return null. However, I'd avoid widening the type until it's absolutely necessary.