Flow: Provide expected type to `document.getElementById`

[TomasBarry]

Proposal

When using document.getElementById, the Flow type returned is a HTMLElement | null. However, it would be reasonable to assume that when using document.getElementById the programmer already knows what type of HTML element they are using, as such, it would be ideal if the return type matches expectation. Something along the lines of:

// Usage
const myElem = document.getElementById<HTMLImageElement>('my_id')

// Changed type definition
getElementById<HTMLElementType>(elementId: string): HTMLElementType

Alternatively, if we did not want to use generics, document.getElementById could return an any and we could type cast:

// Usage
const myElem = (document.getElementById('my_id'): HTMLImageElement)

// Changed type definition
getElementById(elementId: string): any

Although I'm generally not in favour of using any, so a union type could be used instead.

Use case

Problem:

const myImageElement = document.getElementById('my_id')

// assert that `myImageElement` is not `null`
if (!myImageElement) throw new Error('could not find myImageElement')

myImageElement.src = 'some/updated/src'
...

The last line currently raises a Flow error as document.getElementById returns a HTMLElement (if it finds the element) and a HTMLElement does not have a src attribute wheras a HTMLImageElement does.

Solution:

const myImageElement = document.getElementById<?HTMLImageElement>('my_id')

// assert that `myImageElement` is not `null`
if (!myImageElement) throw new Error('could not find myImageElement')

myImageElement.src = 'some/updated/src'
...

[jamesisaac]

Alternatively, if we did not want to use generics, document.getElementById could return an any and we could type cast

You can create this pattern yourself with just a little extra work:

const myElem = ((document.getElementById('my_id'): any): HTMLImageElement)

The benefit of doing it this way is that you're explicitly showing in your code that you're doing something potentially unsafe.

[jamesisaac]

If you wanted to do it safely, I believe you can just refine the myElem type:

if (!myElem instanceof HTMLImageElement) throw new Error('myImageElement unexpected type');

[atsepkov]

@jamesisaac I use that

Alternatively, if we did not want to use generics, document.getElementById could return an any and we could type cast

You can create this pattern yourself with just a little extra work:

const myElem = ((document.getElementById('my_id'): any): HTMLImageElement)

The benefit of doing it this way is that you're explicitly showing in your code that you're doing something potentially unsafe.

I agree with the original poster, I switched to Flow a while back and this is my only gripe with it, as well as similarly related inability to tell Flow that you as a programmer are 100% sure the element will exist on the page and won't be null (TypeScript has ! syntax for it, in Flow you have to double-cast through any, see my SO question on this topic: https://stackoverflow.com/questions/58126869/any-way-to-tell-flow-that-htmlelement-returned-by-getelementbyid-cant-be-null).

I use the above pattern in several places in my code that I wish there was a cleaner way to state it. The above syntax is in my opinion a hack. I don't want to cast through any, it's just visual clutter. The reason I like Flow is that it reduces visual clutter compared to TypeScript. In this case, I wish there was something like :: for potentially unsafe cast instead of (...: any):.

[nmote]

I think the best course of action here would be to cast through any as @jamesisaac suggested, if you want this behavior. In my opinion, the visual noise is a good reminder that this pattern is unsafe, but if you would like to avoid it you could define a helper function.

I don't think it's very likely that we will change this behavior. In general we lean towards safety, even when it causes some inconvenience.

[atsepkov]

@nmote seeing any in the cast chain does not convey to the user that the cast is dangerous, claiming that this "inconvenience" results in safer code is security by obscurity, in my opinion.

[jamesisaac]

@atsepkov Disagree, any has very clear and widely understood semantics: you're bypassing the type system, so Flow can no longer guarantee the code path is safe.

There are many other Flow and third party tool features built on top of this behaviour of any: strict mode, lint rules, coverage reporting, etc. Other unsafe bypasses have been deprecated in favour of any (Object and Function). Whenever any is seen in a Flow codebase it's a very clear sign to the user of a lack of safety.

Don't see the need for adding a shorthand for something that's probably best to be explicit and ideally used infrequently.

[evbo]

for those cases where, like was said, 100% sure, you can always tell Flow to chill out:

// $FlowIgnore[prop-missing] HTMLElement does not have property value
// $FlowIgnore[incompatible-use] HTMLElement might be null
const helper = document.getElementById("contextPath").value