document.body should not require null check #4783

Meai1 · 2017-09-02T07:53:47Z

This kind of null check on document.body should not be required:

var myEl = new MyElement()
if (document.body != null) {
    document.body.appendChild(myEl)
} else {
    console.error("null error")
}

See question: https://stackoverflow.com/questions/42377663/flowtype-constantly-requiring-null-checks

If you dont want to implement this, please tell me how I can do it myself.

The text was updated successfully, but these errors were encountered:

jcready · 2017-09-02T15:13:34Z

Why do you believe a null check shouldn't be required? Nothing in the SO link you provided suggests that should be the case.

Meai1 · 2017-09-02T15:51:10Z

@jcready I dont want to write null checks. It's annoying.

lll000111 · 2017-09-02T18:35:35Z

@Meai1 Then don't. If you don't want something checked there's any to opt out, as well as customizable (default: $FlowFixme) opt-out comments. I don't see why "I don't want a type check" is worth opening an issue in a repo for a tool explicitly made to check types though (and to be strict about it!)... :-)

I too had to learn this: Flow does not work by adding types on top of whatever Javascript code you use to write. It forces you to use - or to not use - certain constructs. After adding Flow you can only use a subset of Javascript code that is possible to write. That is intentional.

Meai1 · 2017-09-02T18:52:53Z

@lll000111 As you can see from my example, I cant exactly add 'any' to document.body. It also defeats the purpose of static typing.
I dont care how Flowtype thinks it has to do it, I want it my way.

lll000111 · 2017-09-02T18:58:49Z

cant exactly add 'any' to document.body.

@Meai1 Please read my short comment, I already answered that.

It also defeats the purpose of static typing.

That too :-)

asolove · 2017-09-02T21:40:48Z

To be clear, it is possible, however unlikely, for a document not to have a body. There are various (again, unlikely but possible) html documents that will do so. The official spec even says this property may return null. So flow is safe and follows this.

Now, if you are sure it will be defined, you can just tell flow that by sssigning it to a variable and giving it a clearer type, perhaps any or at least an html element. But I feel pretty sure flow is not going to change its behavior so I’m going to close this issue.

But you also asked how you could fork flow and code this yourself, which would be quite easy. The code you need to change is just this: https://github.com/facebook/flow/blob/master/lib/dom.js#L558

Meai1 · 2017-09-04T12:48:33Z

@asolove how do I change it so that the change gets picked up in nuclide?

Meai1 · 2017-09-04T13:12:35Z

OK I put all the files from the folder you posted into a project folder called flow-typed, now it works! Thanks!

suchipi · 2018-07-31T18:51:42Z

For people coming here via Google: If you are writing an app where you know document.body will always exist, you can put the following content into a *.js file in the flow-typed folder to change it so you don't need to null check:

class DocumentWithBody extends Document {
  // $FlowFixMe
  body: HTMLBodyElement;
}

declare var document: DocumentWithBody;

If you are writing a library, you should probably keep the null check, since you have fewer guarantees about where it will run.

bsmith-cycorp · 2018-09-11T17:31:38Z

This is yet another Flow productivity-killer that could easily be added as a config option. As far as I know the document body can only be null if your script tag is in the <head> of the document, which has been an established bad-practice for 15 years and which is easy to verify against at a global project level. Setting a config option that says, "No, it's okay, I checked, my script tag is in the right place" would allow the removal of a potentially large amount of complexity from the code itself.

Time and again I see issues like this one where Flow maintainers make impassioned arguments for the purity of the system, accusing users in not-so-many-words of being too lazy to adhere to the noble principles of typing.

I respect a certain amount of that - there are legitimate cases for that type of stance - but it's not just about laziness. Adding a null check every time you reference document.body, when you have the ability to guarantee in another way that it will always exist, is a detriment to code quality. More checks means more code to maintain, means more complexity, means more opportunities for bugs. By taking a hard line against reasonable exceptions like this one, the Flow maintainers are actively making lots of code-bases worse.

OscarBarrett · 2018-10-19T02:20:38Z

Redeclaring document increased the time flow check takes to run from ~6s to ~94s on one of our repos 😕

bsmith-cycorp · 2018-10-19T15:06:18Z

For those trying to cope with problems like this one, I came up with a band-aid:

function given<T,R>(val: ?T, func: (T) => ?R, elseFunc?: () => ?R = () => null): ?R {
  if(val != null) {
    return func(val);
  } else {
    return elseFunc();
  }
}

given(document.body, body => {
  // your logic goes here, and body is no longer a maybe type
})

This will locally-dereference a maybe-value and only execute the callback if the value exists, casting it into a non-maybe type in the process, and returning the result of the callback (or null if the initial value doesn't exist). There's also an optional else callback if you need one. Particularly useful in React render methods.

You can also chain multiple clauses like so:

given(this.someObj, someObj =>
given(someObj.prop1, prop1 =>
given(document.body, body => {
  // logic involving prop1 and body
})))

h-h-h-h · 2018-12-23T14:56:31Z

As the comment on a Stack Overflow answer says, you can use an invariant() function that's like assert(). That'll trigger Flow's type refinement.

This reduces the check to:

invariant(document.body);

Or:

invariant(document.body !== null);

You could view it as kind of a more verbose version of something like:

document.body?.appendChild(el);
//^^^^^^^^^^^^
// (Not generally, but in this case where you're sure that
// `body` is non-null. It's about the type refinement.)

bsmith-cycorp · 2018-12-24T16:04:03Z

I tried to look up "flow assert" and couldn't find any documentation... there's a third-party library called assert on NPM, but I can't say for sure whether 1) it's the one described and Flow just special-cased a random library, or whether 2) the described functionality is supposed to be built-in to Flow and injected in some magic way.

Either way, yikes.

TrySound · 2018-12-24T16:24:49Z

Any call of function with invariant name provides refinement. It's hardcoded like Object.assign and others.There are invariant and tiny-invariant packages which can be used for this.

bsmith-cycorp · 2018-12-24T16:26:15Z

So it's simply any function with the name invariant, no matter what its actual implementation is? Is there any standard or documentation for this or is it as arbitrary as it seems?

TrySound · 2018-12-24T16:27:53Z

It's came from facebook internals I guess and was used by many react packages. There is also babel plugin to remove messages in production and only throw an error.

monolithed · 2019-01-06T14:29:54Z

For people coming here via Google: If you are writing an app where you know document.body will always exist, you can put the following content into a *.js file in the flow-typed folder to change it so you don't need to null check:
class DocumentWithBody extends Document {
  // $FlowFixMe
  body: HTMLBodyElement;
}

declare var document: DocumentWithBody;
If you are writing a library, you should probably keep the null check, since you have fewer guarantees about where it will run.

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ flow/js-stub-interface.js:5:11

Cannot extend Document [1] with DocumentWithBody because HTMLBodyElement [2] is incompatible with null [3] in property
body.

     flow/js-stub-interface.js
       1│ // @flow
       2│
 [1]   3│ class DocumentWithBody extends Document {
       4│     // $FlowFixMe
 [2]   5│     body: HTMLBodyElement;
       6│ }
       7│
       8│ declare var document: DocumentWithBody;
       9│

suchipi · 2019-01-07T00:09:40Z

@monolithed try moving the // $FlowFixMe above the class declaration, or add a new one there

bsmith-cycorp · 2019-01-07T15:39:15Z

@monolithed also keep in mind, as @OscarBarrett stated above,

Redeclaring document increased the time flow check takes to run from ~6s to ~94s on one of our repos 😕

asolove closed this as completed Sep 2, 2017

asolove changed the title ~~Make null checks optional~~ document.body should not require null check Sep 2, 2017

acusti mentioned this issue Jul 17, 2018

Minimally support iframes (nested browsing contexts) in selection event handling facebook/react#12037

Merged

lydell mentioned this issue Sep 22, 2019

Can document.head be made non-nullable? #4565

Open

d-led mentioned this issue Jan 16, 2023

Bug: document.body null causes exceptions when testing with cypress siemens/ix#266

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

document.body should not require null check #4783

document.body should not require null check #4783

Meai1 commented Sep 2, 2017 •

edited

Loading

jcready commented Sep 2, 2017

Meai1 commented Sep 2, 2017

lll000111 commented Sep 2, 2017 •

edited

Loading

Meai1 commented Sep 2, 2017

lll000111 commented Sep 2, 2017

asolove commented Sep 2, 2017 •

edited

Loading

Meai1 commented Sep 4, 2017

Meai1 commented Sep 4, 2017

suchipi commented Jul 31, 2018

bsmith-cycorp commented Sep 11, 2018 •

edited

Loading

OscarBarrett commented Oct 19, 2018

bsmith-cycorp commented Oct 19, 2018 •

edited

Loading

h-h-h-h commented Dec 23, 2018 •

edited

Loading

bsmith-cycorp commented Dec 24, 2018

TrySound commented Dec 24, 2018 •

edited

Loading

bsmith-cycorp commented Dec 24, 2018

TrySound commented Dec 24, 2018

monolithed commented Jan 6, 2019

suchipi commented Jan 7, 2019

bsmith-cycorp commented Jan 7, 2019

document.body should not require null check #4783

document.body should not require null check #4783

Comments

Meai1 commented Sep 2, 2017 • edited Loading

jcready commented Sep 2, 2017

Meai1 commented Sep 2, 2017

lll000111 commented Sep 2, 2017 • edited Loading

Meai1 commented Sep 2, 2017

lll000111 commented Sep 2, 2017

asolove commented Sep 2, 2017 • edited Loading

Meai1 commented Sep 4, 2017

Meai1 commented Sep 4, 2017

suchipi commented Jul 31, 2018

bsmith-cycorp commented Sep 11, 2018 • edited Loading

OscarBarrett commented Oct 19, 2018

bsmith-cycorp commented Oct 19, 2018 • edited Loading

h-h-h-h commented Dec 23, 2018 • edited Loading

bsmith-cycorp commented Dec 24, 2018

TrySound commented Dec 24, 2018 • edited Loading

bsmith-cycorp commented Dec 24, 2018

TrySound commented Dec 24, 2018

monolithed commented Jan 6, 2019

suchipi commented Jan 7, 2019

bsmith-cycorp commented Jan 7, 2019

Meai1 commented Sep 2, 2017 •

edited

Loading

lll000111 commented Sep 2, 2017 •

edited

Loading

asolove commented Sep 2, 2017 •

edited

Loading

bsmith-cycorp commented Sep 11, 2018 •

edited

Loading

bsmith-cycorp commented Oct 19, 2018 •

edited

Loading

h-h-h-h commented Dec 23, 2018 •

edited

Loading

TrySound commented Dec 24, 2018 •

edited

Loading