fix: revert default element typings to HTMLElement

[alecgibson]

The type definitions were updated to allow generic element typing in
#1871

However, this "loosened" the default type from HTMLElement to
`Element. This was actually a breaking change.

For example, consumers may have had this test code:

wrapper.element.click()

But click() only exists on HTMLElement, not on Element, so test
compilation fails.

This change moves the default type back to HTMLElement. If we want to
loosen this requirement in the future, it should be considered a
breaking change.

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes
• No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

• It's submitted to the dev branch.
• When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
• All tests are passing: https://github.com/vuejs/vue-test-utils/blob/dev/.github/CONTRIBUTING.md#development-setup
• New/updated tests are included

If adding a new feature, the PR's description includes:

• A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

[lmiller1990]

Ah, I see.

That said, you can find things other than HTMLElement with find and get (like SVGElement). So while this is breaking change, it's a necessary one, to make the types correct.

Maybe we should revert, then release the "loosened" types as a minor (so 1.3, in this case).

Regarding your example, is wrapper.element.click() a thing? I don't recall an element property on the root wrapper. For other elements, you could do wrapper.find<HTMLButtonElement> etc.

What do you think? I do agree this kind of change is a bit inconvenient, and should not have been a patch release. Even if we revert, you will have this problem when you update to the next minor release (1.3 - we cannot do a 2.x, that's already used for Vue 3).

[alecgibson]

@lmiller1990 Wrapper exposes an element property here in the types and here in the actual code.

The other thing to do to not make this breaking is to just keep HTMLElement as the (sensible) default. Any consumers who need to override the type (eg for SVG elements — don't know how many other examples there are?) can do so explicitly (as they would need to anyway, if they need SVG-specific methods, etc.).

That being said, I can understand if this is what you want to do to be more "correct". Just a bit surprising on a patch version in particular.

As a side-note, even Microsoft's official type definitions seem to be a little bit confused on the matter of casting verbosity vs spec compliance: microsoft/TypeScript#4689

[lmiller1990]

@alecgibson seems VTU v2 (for Vue 3) is using Element https://github.com/vuejs/vue-test-utils-next/blob/master/src/baseWrapper.ts#L7. We should make them consistent, seems like Element is the best choice, but I definitely see why this was frustrating - sorry about that.

I agree doing this as a patch release was a bad idea, it should definitely have been a minor release. I'm not sure there's value in reverting it, only to re-release it as a minor in the near future.

For get and find - I wonder if we could just use the types from VTU v2 for get and find: https://github.com/vuejs/vue-test-utils-next/blob/master/src/interfaces/wrapperLike.ts - they seem a lot more well defined.

[alecgibson]

Yes definitely a shame; this will definitely make the TS syntax for this library pretty verbose — (lots of wrapper.find<HTMLElement>() everywhere), but if that's how you all want it then it's a valid choice.

[lmiller1990]

Hang on - why can't we just rework this PR to use the similar types from VTU v2?

It can actually infer the element type, eg:

mount({}).find('div').element.click()

What do you think? It won't work with #selector, though. So maybe it's not that useful. I do think that Element is more correct, and we should go with it moving forward. Apologies on the unexpected breaking change.

[alecgibson]

Yes it's fine if you reference by element tag, but I personally tend to avoid that to keep tests from being too fragile (and often looking things up by just eg div are usually not specific enough).

The scope of this PR was just to make the change un-breaking; so if you want to adopt the v3 types, that should probably be a different change.

[lmiller1990]

I understand now. Fair enough - I agree that this should not have been a patch, definitely not ideal. Happy to discuss more on general type improvements, the Vue 3 lib is a good place to look for ideas since we also have a tsd test suite there to validate the types. If we do want to make a change like this, we should at least make it consistent across both libs. In this particular case, I think Element is the best choice, although I do agree it's a bit verbose. Thanks for your input and understanding.

[alecgibson]

As a side-note, it sounds like using this sort of generic typing where the type isn't used in the parameter list is an anti-pattern, and I guess now I think about it, I don't really understand the value of doing this over just forcing consumers to cast:

const button = wrapper.find('.button') as HTMLButtonElement;

[lmiller1990]

Hm, I don't fully understand "Type parameters that aren't used in the parameter list are an anti-pattern". Why is this an anti pattern? Does this mean we should just not have any generic here at all? I could be misunderstand this. If we have made a bad decision with this type, we can change it. We should have a good reason as to why, and make sure V2 also has the same types, or it'll be hard to upgrade.

I think the difference between casting with as HTMLButtonElement and the generic is when using a generic, we can enforce you must pass something that extends Element. Using as XXX allows you to write any type, which might not be valid.

[alecgibson]

Why is this an anti pattern?

I think it's an anti-pattern, because a purist would argue that this use of generics offers no "real" type safety. For example, a classic example of a generic might be the Array:

const arr = new Array<number>();
arr.push(1);
const item: number = arr.pop();

The generic here offers type-safety, because it ties everything about the class together: you definitely can only add a number, and always are guaranteed to get a number.

If the Array (hypothetically) let you override its generic types on its instance methods, you could write something like:

const arr = new Array(); // Defaults to any
arr.push<number>(1);
const item = arr.pop<string>();

...then you've not gained anything from the type system, and you're just abusing the fact that the methods let you override the expected type — essentially you've just cast the result — which then begs the question of what value is the type system adding here.

If we follow our wrapper.find<HTMLElement>() {} to its logical extreme, it's akin to writing:

function foo<T extends any = any>(): T {}
const bar = foo<string>();

...which really offers no more type safety than this:

function foo(): any {}
const bar = foo() as string;

Using as XXX allows you to write any type, which might not be valid

That is not the case:

However, all that being said, as a pragmatist, I do think if we're going to be forced to constantly cast our find() results, then this syntax:

wrapper.find<HTMLElement>('.foo').element.click()

is arguably "nicer" than this syntax:

(wrapper.find('.foo').element as HTMLElement).click()
// Or alternatively...
(<HTMLElement> wrapper.find('.foo').element).click()