Flow: Flow Requires Redundant Check For Object Existence

[cyorobert]

Sorry if this has been asked before, but I have a function that checks for the existence of document.body

export const downloadFromLink = (uri : string, file_name : string) : void => {
    const link      = document.createElement('a')
    link.href       = uri
    link.download   = file_name
    if(document.body) {
        document.body.appendChild(link)
        link.click()
        document.body.removeChild(link)
    }
}

In this case, I've checked for the existence of document.body, so I should be ok to call both the appendChild and removeChild methods inside of the if block. However, I get there error from flow:

call of method 'removeChild'. Method cannot be called on possibly null value null

Thus, to avoid errors in my types I've had to rewrite the function (below), which is redundant.

export const downloadFromLink = (uri : string, file_name : string) : void => {
    const link      = document.createElement('a')
    link.href       = uri
    link.download   = file_name
    if(document.body) {
        document.body.appendChild(link)
        link.click()
        if(document.body) {
            document.body.removeChild(link)
        }
    }
}

Is this a known issue or expected behavior? I'm using IntelliJ with flow if that is relevant. It's one of the language settings for javascript in the IDE and provides these warnings as tooltips.

[jcready]

You can get around this by creating a reference to document.body:

const body = document.body
if (body) {
  body.appendChild(a)
  link.click()
  body.removeChild(a)
}

[Andarist]

@cyorobert ive stumbled on the similar issue before - its because flow doesnt know if your link.click() didnt remove body property from the document object

[jcready]

@Andarist I am curious, why doesn't it make the same assumption about link.click() removing the removeChild() method from body?

[cyorobert]

@Andarist That's a good point I hadn't thought of that. I wonder if this should produce a warning rather than an outright error though. An error seems extreme when flow doesn't actually know if any mutation occurred.

[Andarist]

@jcready truth to be told, im no flow expert, still learning and truth to be told im not yet quite comfortable using flow, im covering with types https://github.com/yelouafi/avenir for the last 2 days and still hasnt finished, also ive faced many issues which i dont understand - unfortunately there are limited online resources on the web

ive only spotted this issue by accident, when trying to search for details about refinements and it immediately resembled the one I have faced here - https://github.com/facebook/flow/issues/4155#issuecomment-308199137

why it doesn't make the same assumption about link.click() removing the removeChild() method from body?

honestly I dont know, maybe body is somehow frozen and marked as read only? dunno why document would be treated different

[calebmer]

I recommend going with @jcready’s approach. The reason this happens is that link.click() may be implemented as:

link.click = () => {
  // Muwahahaha!
  document.body = null;
};

While this is incredibly unlikely, it is possible, so Flow is helping you by letting you know that there might be an error. We could say that document.body is read only in our types, but document.body is assignable so that would not make sense: https://developer.mozilla.org/en-US/docs/Web/API/Document/body

So instead I recommend that you write:

const body = document.body;
if (body) {
  body.appendChild(a);
  link.click();
  body.removeChild(a);
}

Like @jcready suggested 😊

[cyorobert]

If anyone else stumbles on this issue this is what I'd recommend reading https://flow.org/en/docs/lang/refinements/#caveats.

[Andarist]

@cyorobert I think the section u wanted to point to is under this URL https://flow.org/en/docs/lang/refinements/#refinement-invalidations

@calebmer could u also answer on @jcready 's comment? would be really helpful

@Andarist I am curious, why doesn't it make the same assumption about link.click() removing the removeChild() method from body?

[cyorobert]

@Andarist You are correct - that's the right link.

[calebmer]

removeChild() is not a union. Even if you tried removing removeChild() from body you couldn’t say you wrote:

document.body.removeChild = null;

Flow would error because removeChild() cannot be null! However, body can be null. Good question @jcready 😊