Possible to narrow type of object literal property with strictNullChecks?

[thw0rted]

TypeScript Version: Current playground (2.6?)

Code

type Foo = {
  bar?: number[];
}

let f1: Foo = {};
f1.bar = [];
f1.bar.push(0);

let f2: Foo = { bar: [] };
f2.bar.push(0);

Expected behavior:

Both references to .bar compile successfully.

Actual behavior:

Second reference (f2.bar.push(0)) fails with "object is possibly undefined" -- the type of f2.bar is still treated as number[] | undefined even though it obviously can't be undefined.

---

This might not be an actual bug as such but whatever magic allows the compiler to narrow the type of f1.bar after assigning to it, should also be able to narrow the type when it was assigned in the literal.

[aluanhaddad]

IMO I think it would be better if in

let f2 = { bar: [] };

The type of f2.bar was not inferred as never[].

[RyanCavanaugh]

@sandersn can you give this a shot and see if it's super hairy? The proposal is only to handle this (i.e. treat property assignments as assignments to properties) when a variable has a bare object literal initializer. IOW we are willing to Won't Fix this if it's going to be a ton of extra code / perf hit.

[sandersn]

I have a prototype in the branch narrow-by-property-assignment. The code is not too hairy at the moment, although it’s not properly recursive yet. But it adds control flow nodes at every PropertyAssignment so I suspect it will have a lot of overhead in the checker. I’ll report back after testing it.

Here are its other limitations:

1. Requires its own FlowFlag because isMatchingReference assumes that it's operating only on references or variable declarations, not initializers.
2. Has a bunch of ugly plumbing in the binder's bindAssignmentTargetFlow; probably it could be simplified.
3. Doesn't add or check control flow nodes recursively, so only top-level property initialisers narrow.
4. Doesn’t handle ShortcutPropertyAssignment.

[sandersn]

Well, there's no significant change in performance, so I'll try cleaning up and hardening the change when I have time.

[sandersn]

I realised yesterday that none of our performance tests have --strict and only one, the union-based compiler, use unions much at all. I'll need to switch some of the test cases to --strict and rebaseline.

[sandersn]

Later: looks like there is still no performance impact, even with --strict on.

[sandersn]

Fixed by #22006

[MartinJohns]

@sandersn No update on this issue, the PR is closed due to inactivity, but the status is committed and the milestone is set to TypeScript 3.1 (after regularly bumped).

[DanTup]

Does this issue cover the below? The mention of object literal in the title makes me unsure, but I don't want to open a new issue if it's covered.

Here, I have a function that takes a an object that has a property declared with | undefined (since it can be), but I want to pass it to functions that require it is defined (their signature includes & { value: string { to disallow undefined).

Currently I have to force a cast with as (included here in a comment), but this makes me sad, since if something changes this code could become incorrect. Is there a way to get TS to automatically know that the property cannot be undefined at that point? (And if not currently, is it covered by this issue or should I open a new one?).

interface ThingWithString {
    readonly value: string | undefined;
}

function printString(a: ThingWithString & {value: string}) {
    console.log(a.value);
}

function getMeString(): ThingWithString {
    return { value: "String" };
}

const maybeString = getMeString();
if (maybeString.value) {
    printString(maybeString /*as ThingWithString & {value: string}*/);
}

[sandersn]

@DanTup no, you have a different problem, which is that Typescript will only narrow union types. In fact, there is no type ThingWithDefinedString { readonly value: string } in the program, and the compiler would have to create one to get the behaviour you want. That's too expensive for it to do, unfortunately.

[DanTup]

@sandersn I see. Is there any safer way to do what I want than as ThingWithString & {value: string} (which seems risky if I ever changed it, that could become incorrect)? I've found myself doing this in quite a few places where I have some type that has optional fields, but functions that require that data. I already have checks to ensure it exists, but I can't find a clean way to make it work.

[DanTup]

Looks like this might work... it's not perfect, but maybe better than the as (I don't know if there are perf implications, but for the places I'm doing this that's not a concern):

if (maybeString.value) {
	printString({ ...maybeString, value: maybeString.value });
}