CheckJS cannot cast Element to HtmlElement types

[mjbvz]

TypeScript Version: 2.4.2

Code
For this JavaScript with checkJs enabled:

// @ts-check
/**
 * @type {HTMLImageElement}
 */
const el = document.querySelector("#definitelyAnImage");
el.src = "image.png";   

Expected behavior:
Can cast result of document.querySelector which returns an Element to an HTMLImageElement

Actual behavior:
Error:

file: 'file:///Users/matb/projects/san/a.js'
severity: 'Error'
message: 'Type 'Element' is not assignable to type 'HTMLImageElement'.
  Property 'align' is missing in type 'Element'.'
at: '5,7'
source: 'js'

[weswigham]

/**
 * @type {HTMLImageElement}
 */
const el = document.querySelector("#definitelyAnImage");

currently is equivalent to

const el: HTMLImageElement = document.querySelector("#definitelyAnImage");

which will error, as the types are incompatible.

However, in nightly we should now support

const el = /* @type {HTMLImageElement} */ (document.querySelector("#definitelyAnImage"));

(JSDoc casts attached to parenthesized expressions)
which is equivalent to

const el = (document.querySelector("#definitelyAnImage") as HTMLImageElement);

which is actually a cast and not an assignment, and I think matches up better with the answer this issue was looking for.

[jcready]

Why would you expect to be able to do this without refinement checks? I'm more curious why document.querySelector isn't returning ?HTMLElement since the thing you're querying for might not actually exist.

[weswigham]

@jcready Actually, it returns specifically null, and not undefiend. You'd probably need strict null checks on to notice in the type system, though. But anyway, it returns Element and not HTMLElement because the interface is generic to what document its used with - it can validly be used to select xml nodes which do not have the html element properties.

[jcready]

Ah, sorry about the HTMLElement that was just a mistake. I meant ?Element. But yeah, your explanation makes sense. So if the actual return type is essentially Element | null shouldn't OP's example also fail because it doesn't null check before setting el.src?

[weswigham]

@jcready Only if the strictNullChecks compiler flag is true.

[thw0rted]

Hi, OP here -- just to clarify, in the 1.14.x, the answer is that you can't fix this, but at some point in the future there will be an inline comment syntax to do type coercion?

A thought just occurred to me, but I'm not at work so I don't have an easy way to test it. Would TS be smart enough to pass this?

let el = document.querySelector("#definitelyAnImage");
if (el.constructor === HTMLImageElement) {
  el.src = "foo.png";
}

It's extra verbose, and I happen to know that in my code the if-check would always pass, but it has the distinct advantage of being 100% vanilla JS without any TS-specific cruft. (The check could be any kind of runtime type checking, typeof matching, instanceof, whatever.)

[weswigham]

@thw0rted

var x = document.querySelector("#foo");
if (x instanceof HTMLImageElement) {
    x.src = "whatever";
}

Is already interpreted and typechecked correctly today.

[thw0rted]

This mostly works, but falls apart in closure scopes:

var x = document.querySelector("#foo");
if (x instanceof HTMLImageElement) {
    x.src = "whatever.png"; // works
    x.addEventListener("click", () => {
        x.src = "another.png"; // error, Element does not have property .src
    });
}

I can of course put another "if ... instanceof" inside the anonymous function but it starts to get a little ridiculous. The only good fix here is to provide a dynamic cast syntax as @weswigham stated (the one that works in Nightly). I started adding instanceof checks all over my code, but I think I might switch to the inline @type comments and just try to ignore the errors until support comes to the production build.

[kitsonk]

@thw0rted the callback functions are #17449, #11498 and #9998. There are tradeoff with CFA and at the moment, function scope reset guards because the compiler can't be certain the variable hasn't changed.

[thw0rted]

Does the JS checker not respect const? I should note that while the example above uses var x I saw the same behavior with const x, where checking instanceof once should "clear" the whole scope, including nested scopes.

[kitsonk]

It does in certain ways, but to the compiler an instanceof guard is like any other guard, versus the reality. The reasons for this are complex (see: #202) in that the rest of JavaScript behaves structurally, though instanceof is essentially a nominal typing operator. So the compiler resets guards inside of functions, thinking that the variable could have its structure change (though in this case, not in a way that would have invalidated the instanceof narrowing).

[thw0rted]

I found another issue with this. foo instanceof SomeClass causes type narrowing but foo.constructor === SomeClass does not; I believe both are asserting the same thing. This is important. If there's a good way to narrow the type to "is a constructed String or string literal" I haven't found it.

ETA: this applies to any primitive types, not just String

[kitsonk]

For primitives, why are you not using the typeof operator?

As for narrowing foo based on one of its members, there isn't that sort of co-dependent type narrowing in the language, though there have been a couple of proposals (see: #12424 and #13257)

[thw0rted]

I'm just trying to avoid horribly verbose type checks, and at least in my environment I've found that constructor testing works every time and doesn't require multiple checks for edge cases.

let s1 = new String("foo");
let s2 = "foo";
s1 instanceof String; // true
typeof s1; // "object"
s1.constructor; // String
s2 instanceof String; // false
typeof s2; // "string"
s1.constructor; // String

Note that .constructor is the only thing that's the same both times. Is there a reason we can't trust that Object.prototype.constructor correctly identifies the type of the thing?

[kitsonk]

IMO, it will be a long time to get the sort of co-dependent member checking that you need.

Being practical, using custom type guards would be how I would solve this in TypeScript. I am not sure though how this would work in Salsa (and how much you are willing to restructure your code). In TypeScript you can pretty much contain any sort of "advanced" typing logic in a function, like you could do:

function isString(value: any): value is string {
    return value.constructor === String;
}

let s1 = new String("foo");
let s2 = "foo";
isString(s1); // true
isString(s2); // true

[thw0rted]

I would not be averse to having a custom type-narrowing function, if that's possible in Salsa. Maybe I need to put in a new issue to support some kind of (comment-directive-based) type guard syntax for JS functions? I'm assuming that everything in #1007 is TS-only and doesn't have a JS equivalent?

[mhegazy]

this should be working as intended now in latest. a few notes:

This works because querySelector is a generic function, and with inference from return types added in TS 2.4, the return type would be inferred to be HTMLImageElement.

/**
 * @type {HTMLImageElement}
 */
const el = document.querySelector("#definitelyAnImage");
el.src = "image.png";   

you can alternatively cast in place using JSDoc @type as:

const el = /** @type {HTMLImageElement} */( document.querySelector("#definitelyAnImage"));
el.src = "image.png";   

[rohmanhm]

This worked for me :)

(document.querySelectorAll('.primary-menu > ul > li') as any as HTMLLIElement[])

[thw0rted]

@rohmanhm the original issue was about how to assert return types in vanilla JS (through tsc), not TS, so as is not an option in that case. The inline JSDoc-based example above is the way to go, I think.