Interface Index Signatures + Strict Null Checking

[blakeembrey]

TypeScript Version:

Version 1.9.0-dev.20160617-1.0

Code

interface Foo {
  prop?: string
  [key: string]: string
}

{
  "compilerOptions": {
    "strictNullChecks": true
  }
}

Expected behavior: No error. Wouldn't the fact that it's an index signature implicitly signal that it could also be undefined?

Actual behavior:

index.ts(2,3): error TS2411: Property 'prop' of type 'string | undefined' is not assignable to string index type 'string'.

Edit: On the other side of the coin with this usage:

interface Foo {
  [key: string]: string
}

const foo: Foo = {}
const bar = foo['test']

Should bar be type of string | undefined?

[ahejlsberg]

Yes, you could argue that every index signature should have undefined added to its type, but it would break the very common case of arrays and you can always add a ? yourself. See conversation here.

[blakeembrey]

Anders Hejlsberg (@ahejlsberg) I didn't think you could add ? to an index signature. The things that tricked me up here is that if you take a definition generated when using strictNullChecks: false and try to use on a system that strictNullChecks: true, it'll error. This seems like a point where there'd be a lot of back and forth between a module author and consumers of the module.

I do see the argument about the array case, but I feel like you'd catch a lot more user issues with this check. Eventually, flow control could even detect that i is between 0 and arr.length and know you aren't accessing something out of range, right? (Assuming the main array index case you mention is a loop).

However, isn't this a similar case as Object.keys() now? You could do the same loop on Object.keys(). Seems like there's not much benefit to the bugs it could be catching and errors it could avoid. New users can be pointed to use for..of.

And because I do feel it's going to be painful, anyone that has written code with an index signature today will break on 2.0 immediately with strictNullChecks: true. That could be a decent chunk of TypeScript on NPM.

Edit: Also, strictNullChecks is a toggle that's disabled by default - if someone enables it, I'm sure they expect to catch things like this and won't find it that irritating.

[rkirov]

This issue has an "interesting" consequence - a compilation with tsc test.ts --declarations (no strictNullChecks) can produce a .d.ts file that is internally invalid when used in a following compilation. When calling tsc test.d.ts more.ts --strictNullChecks one sees the error above, but it has nothing to do with the code at hand.

So far this is the only way that I know of that happening, i.e. the validity of the .d.ts being dependent on the strictNullChecks flag. It would be nice to have the property that independent of the compilation flags, produced .d.ts files are valid in any compilation downstream (even if they don't describe the API completely as described in #9655).

[ewinslow]

If the concern is just breaking folks who have --strictNullChecks on, could this be considered for something like a 3.0 (intended breaking change)?

Reposting the rest of my response from #11122 since that was closed as dupe and I am bummed about the current decision:

+1 for fixing this. I've been bitten by this compromise a couple times now so count me in for more safety not less!

the only practical result is that xs[i]! gets written everywhere instead

If folks are using an unsafe pattern everywhere, maybe that's exactly what should happen. I don't find it particularly onerous. I guess I'd like to see some data on exactly how much impact this would have on existing TypeScript programs before we write it off as making the language unusable.

• how many real bugs it would find
• how many false positives
• how much it costs to annotate the false positives w/ ! (this could be automated for existing code)

Right now the sentiment I've seen from TS team is that 1 is negligible, 2 is huge, and 3 is prohibitive. Are there numbers to back up these hunches?

Some alternatives to ! proliferating:

• Do undefined-checks on the result. This is the right thing to do in many cases!
  Rewrite loops with for (let item of array) or array.forEach instead of for(let i = 0; i < array.length; i++).
• Looping over a string[] array with for-of or forEach (or map, etc.) would never emit undefined, since each result is guaranteed to be in-bounds.

The same could be argued for Map<string,string>. map.get('foo') returns string | undefined, but map.set('foo', undefined) would be banned so that when looping over values with forEach or somesuch, you are guaranteed only to get string values, not string | undefined. And indeed, that is the current behavior of Map.

[OliverJAsh]

If authors enable strictNullChecking then they should expect some boilerplate overhead in return for increased safety. I'm surprised TypeScript isn't consistent in its approach here—as far as I know, this is the only scenario where the team has decided to go against type safety in favour of reduced boilerplate with regards to strictNullChecking?

I'm not too familiar with many other type systems, but Scala does have List.headOption which will return Option<T>, which would be the same thing as T | undefined in TypeScript.

Indeed there would be slightly more boilerplate if we did return T | undefined, as discussed above, but surely the job of the type system is to return the correct type, so the author can handle that correctly.

Without this there will be a whole class of bugs that TypeScript will never catch. 😒 😢

[Flarna]

Maybe there is the need to distinguish somehow between properties which are optional and properties which are present but may have undefined as value?
Currently it seems to me like this is the same for TypeScript but not not for JavaScript.

[gcnew]

Gerhard Stöbich (@Flarna) It actually does. Both for functions and objects.

type Bag = {
    optional?: number;
    mustBePresent: number|undefined;
}

const bag: Bag = {
    mustBePresent: 3 // type error if missing
    // we dont need to specify `optional`
};

function f(mustBePresent: number|undefined, optional?: number) {}

f(undefined); // OK
f(15, undefined); // OK
f() // not OK, `mustBePresent` is missing

[Flarna]

Sorry, seems I was not clear with my comment. If I have a type like that

type A = {
    a?: number;
    b?: number;
}
const a: A = {
    a: undefined,
    b: 15
};

I would expect that I can iterate over all properties and get only numbers. But actually I may get also undefined.
So the ? tells that the property may be missing and may have the value undefined.

If an index signature is used this is different:

type B = {
    [key: string]: number
}
const b: B = {
    optional: undefined   // property optional is incompatible....
}

Here the properties are optional but if they are present they are numbers.

[Strate]

Maybe it could be handled with extra compiler option?
In runtime calling array[index] or object[propName] definetely could return undefined if index is out of bounds or propName does not exist on object. It is really good to force to check returned that value before using it.

[MartinJohns]

This really seems like a half-hearted solution. I expected the strictNullChecking flag to warn me about potential undefined in my code - but in the case of indexers it let me down. This is extremely irritating.