Enable strictNullChecks

[schmidt-sebastian]

Hmmm.

(BTW: This is the dumb version of this PR. There are likely asserts we could add to make sure that our assumptions are valid).

[wilhuff]

I think we could add an assertNotNull type guard that that would make the ! below unnecessary.

Something like:

assertNotNull<T>(value : T | null, explanation: string): arg is T {
  assert(!!value, explanation);
  return true;
}

[schmidt-sebastian]

Since these type guards only work in if-statements, this would change the calling site to:

  if (assertNotNull(query, `Query data for active target ${targetId} not found`)) {
        const queryData = await this.localStore.allocateQuery(query);
        await this.initializeViewAndComputeSnapshot(
            queryData,
            /*current=*/ false
        );
        this.remoteStore.listen(queryData);
      };

If we didn't have to put the explanation, then I would be all in favor. But with the explanation this looks a tad strange. If we do drop the explanation, we will never be able to trace these asserts back in minified code.

[wilhuff]

Shouldn't this be ==? (We test event.newValue != null just below.)

[schmidt-sebastian]

I'm surprised that the linter didn't complain about event.newValue != null. We should not rely on type coercion, and I would guess that implicit coercion is almost always unintended. So rather than using == here, I changed the line below to !==. The type for LocalStorage are defined as "string | null", so they should never be undefined.

[wilhuff]

I think the issue is that == null is the one case where double-equal is actually useful because it handles undefined in a sane way. I don't actually know which we prefer, but ISTR this point being discussed.

[wilhuff]

Is something wrong with our typings? Would our end users have to do something like this to enable strict null checks?

If not why do we need to do this?

[schmidt-sebastian]

I asked Josh about this yesterday. The way our types are defined allows us to extend the firebase namespace after the fact without having to add all other product's APIs as well. Note that we don't ship generated typings, but only the hand-curated types in firestore-types and firebase-types.

[wilhuff]

LGTM