RFC: "@cannotBeUndefined" for optional properties

[octogonz]

Suppose an interface has optional members (possibly introduced by Partial<T>). For example:

class Person {
    public name: string = '';
    public age: number | undefined = undefined;

    public constructor(traits: IPersonTraits = {}) {
        this.update(traits);
    }

    public update(traits: IPersonTraits) {
        if (traits.name) {
            this.name = traits.name;
        }
        if (traits.hasOwnProperty("age")) {
            this.age = traits.age;
        }
    }
}

interface IPersonTraits {
    /**
     * The person's name or `""` if not known.
     */
    name?: string;

    /**
     * The person's age, or `undefined` if not known.
     */
    age?: number | undefined;
}

TypeScript currently allows undefined to be specified for both IPersonTraits.name and IPersonTraits.age. In other words, name?: string is equivalent to name?: string | undefined, and in fact gets normalized to that in the emitted .d.ts file. This is a gap in the type system. (See microsoft/TypeScript#13195 which proposes to improve the language to distinguish between a missing member versus an undefined value.)

This gap can lead to bugs. For example:

let person = new Person();

// CORRECT USAGE:
person.update({
    name: 'hello',
    age: undefined  // <-- overwrites "age" with undefined, as intended
});

// MISTAKE:
person.update({
    name: undefined,  // <-- assigns undefined to "name", which is not an allowed value
    age: 13
});

The compiler cannot catch this mistake, and in fact the shipping .d.ts files will contain the normalized name?: string | undefined declaration, which misleadingly implies that undefined is a meaningful value for name.

A possible solution

Suppose we introduce a TSDoc modifier tag call @cannotBeUndefined, which would be used like this:

interface IPersonTraits {
    /**
     * The person's name or `""` if not known.
     * @cannotBeUndefined
     */
    name?: string;

    /**
     * The person's age, or `undefined` if not known.
     */
    age?: number | undefined;
}

Benefits:

• The tag will be appear in the normalized .d.ts file
• The tag will be displayed in the VS Code tooltip, providing clear documentation
• We could implement a TSLint rule that looks for this tag, and if found, reports an error for an expression that is possibly undefined

What do you think? Does this seem useful enough to be a standard tag?

[Gerrit0]

let person = new Person();
// MISTAKE:
person.update({
    name: undefined,  // <-- assigns undefined to "name", which is not an allowed value
    age: 13
});

Are you sure this is a mistake? I don't think it is. Running this code gives me a person whose name is still "" if I change the update code to cause this bug by using hasOwnProperty like for age, then the compiler correctly reports an error.

There is a bug with this similar version: Playground

class Person {
    public name: string = '';
    public age: number | undefined = undefined;

    public constructor(traits: IPersonTraits = {}) {
        this.update(traits);
    }

    public update(traits: IPersonTraits) {
        if ('name' in traits) {
            this.name = traits.name;
        }
        if (traits.hasOwnProperty("age")) {
            this.age = traits.age;
        }
    }
}

type IPersonTraits = { age?: number } | { age?: number, name: string }

let person = new Person();
// MISTAKE:
person.update({
    name: undefined,  // <-- assigns undefined to "name", which is not an allowed value
    age: 13
});

console.log(person)

... but that isn't caused by this problem, but by how TS checks if the object is assignable.

[octogonz]

You're right, for the behavior I described, the implementation really should have been:

        if ('name' in traits) {
            this.name = traits.name;
        }
        if ('age' in traits) {
            this.age = traits.age;
        }

But if we kept my original implementation, then it's still arguably a bug, just a different one:

// MISTAKE:
person.update({
    name: undefined,  // <-- has no effect at all, which is counterintuitive given how "age" works
    age: 13
});