Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distinguish missing and undefined #13195

Closed
dgoldstein0 opened this issue Dec 28, 2016 · 62 comments · Fixed by #43947
Closed

Distinguish missing and undefined #13195

dgoldstein0 opened this issue Dec 28, 2016 · 62 comments · Fixed by #43947
Labels
Breaking Change Would introduce errors in existing code Suggestion An idea for TypeScript

Comments

@dgoldstein0
Copy link

dgoldstein0 commented Dec 28, 2016

TypeScript Version: 2.1.4

Code

Current, with --strictNullChecks on, the typescript compiler seems to treat

interface Foo1 {
  bar?: string;
}

and

interface Foo2 {
  bar?: string | undefined;
}

as being the same type - in that let foo: Foo1 = {bar: undefined}; is legal. However, this does not seem to be the original intent of the language - while some people have used ? as a shortcut for | undefined (e.g. #12793 (comment)), it seems that the intent of the operator was to signify that the property either did not appear, or appeared and is of the specified type (as alluded in #12793 (comment)) - which might not include undefined - and in particular it would let us write types that either have a property that, if enumerable is not undefined, or is not present on the object.

In other places it might make sense for ? to keep serving as a shorthand for | undefined, e.g. for function parameters. (or maybe for consistency it would make sense to stick with it meaning "always pass something undefined if you pass anything"? not sure what's best there. Happy to discuss.)

Expected behavior:

interface Foo1 {
  bar?: string;
}
function baz(bar: string) {};

let foo: Foo1 = {bar: undefined}; // type error: string? is incompatible with undefined

if (foo.hasOwnProperty("bar")) {
  baz(foo.bar); // control flow analysis should figure out foo must have a bar attribute here that's not undefined
}
if ("bar" in foo) {
  baz(foo.bar); // control flow analysis should figure out foo must have a bar attribute here that's not undefined
}

Actual behavior:

interface Foo1 {
  bar?: string;
}
function baz(bar: string) {};

let foo: Foo1 = {bar: undefined}; // compiler is happy

if (foo.hasOwnProperty("bar")) {
  baz(foo.bar); /*  error TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.  Type 'undefined' is not assignable to type 'string'*/

}
if ("bar" in foo) {
  baz(foo.bar); /* error TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.  Type 'undefined' is not assignable to type 'string'*/
}
@felixfbecker
Copy link
Contributor

felixfbecker commented Dec 28, 2016

I agree, in JS there is a difference between the two

const a = { prop: undefined };
a.prop // undefined
a.hasOwnProperty('prop') // true
'prop' in a // true
for (k in a) {
  console.log(k) // logs 'prop'
}

const b = { };
b.prop // undefined
b.hasOwnProperty('prop') // false
'prop' in b // false
for (k in b) {
   console.log(k) // never hit
}

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Dec 28, 2016
@masonk
Copy link

masonk commented Dec 29, 2016

I wrote that comment you referenced. I go back and forth on this. The devil's advocate position is that, under the proposed stricter interpretation of ?, I'll still be able to assign {} to Foo, and thus, I still must treat foo.bar as string | undefined at all places in my code. The only thing I can see being achieved with this higher level of strictness is a new way of type guarding a value for which a perfectly good type guard is already available.

Instead of looking at keys, you can accomplish the type inference you want right now by looking at values.

interface Foo {
  bar?: string;
}
function baz(bar: string) {};

let foo1: Foo = {bar: undefined}; // would become type error: string? is incompatible with undefined
let foo2: Foo = {}; // Would still, however, be allowed

if (typeof foo1.bar !== 'undefined') {
    baz(foo1.bar); // Control flow already correctly narrows type to string
    baz(foo2.bar); // Control flow correctly warns that this could be undefined (under strictNullChecks)
}

So, if there is a specific key in mind, there is already a clean way of type guarding it.

Is the difference more pertinent when enumerating a whole object by keys? I couldn't construct a case in which it is.

for (let k of foo) {
    let val = foo1[k]; // Would still be any under strictOptionalMember, and still an error under noImplicitAny
}

Possibly some meaningfully different expression could be constructed using lookup types. But I almost think it's a code smell to care about whether a key exists on an object.

The bird's eye view is that JavaScript intends for the developer to equivocate between "no key" and "key: undefined". Thinking about the existence of a key is generally not a very useful paradigm in JavaScript. Instead, what really matters are the possible types of the values. So while this new way might be more faithful to the mechanics of JavaScript, it's not more faithful to the intended usage of JavaScript.

@felixfbecker
Copy link
Contributor

felixfbecker commented Dec 29, 2016

I have met JavaScript libraries that used 'prop' in options or options.hasOwnProperty('prop') to check for the presence of an option, and then when you pass undefined (because you just pass your own optional parameter for example) it results in bugs TypeScript didn't catch.

@masonk
Copy link

masonk commented Dec 29, 2016

That's a strong argument. It's good for TypeScript's type system to be able to express whatever is possible in JS, whether or not those possibilities are a good idea.

@dgoldstein0
Copy link
Author

yeah... if I was designing javascript from scratch, I'd probably leave out the whole concept of undefined. But it exists in js, and I don't think typescript is willing to try to change that, so we have to live with it. Making it possible to distinguish, in the type system, between an attribute existing or not without making that attribute optionally undefined would be useful, since we are working with a language that has these (perhaps unnecessary) complexities.

@KiaraGrouwstra
Copy link
Contributor

The current missing/undefined convolution does seem potentially problematic...

const x: {a?: string} = {a: undefined}; // expect error, but passed

@jcalz
Copy link
Contributor

jcalz commented Aug 29, 2017

Is it possible to introduce void to mean "missing" and leave undefined as undefined? So { foo?: string } would be equivalent to { foo: string | void }?

@OliverJAsh
Copy link
Contributor

Where does this issue currently stand? Is it something under consideration, or are there reasons why we can't fix this?

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Mar 14, 2018

Update: separate issue with reduced test case #35983

Another example where TypeScript's inability to distinguish missing from undefined leads to inconsistencies between the types and JavaScript's actual behaviour is usage of object spread.

A real world example of this pattern is a withDefaults higher-order React component.

type Props = {
    foo: string;
    bar: string
}

type InputProps = {
    foo?: string;
    bar: string;
}

const defaultProps: Pick<Props, 'foo'> = { foo: 'foo' };
const inputProps: InputProps = { foo: undefined, bar: 'bar' };

// Type of `foo` property is `string` but actual value is `undefined`
const completeProps: Props = { ...defaultProps, ...inputProps };
$ node
> const defaultProps = { foo: 'foo' };
undefined
> const inputProps = { foo: undefined, bar: 'bar' };
undefined
> const completeProps = { ...defaultProps, ...inputProps };
undefined
> completeProps
{ foo: undefined, bar: 'bar' }

Ideally, this would throw a type error:

// `{ foo: undefined; bar: string; }` is not assignable to `{ foo?: string; bar: string; }`
const inputProps: InputProps = { foo: undefined, bar: 'bar' };

On the other hand, if TypeScript did purposely not distinguish between missing and undefined, TypeScript should instead emit an error when spreading:

// `{ foo: string | undefined; bar: string }` is not assignable to `Props`
const completeProps: Props = { ...defaultProps, ...inputProps };

@OliverJAsh

This comment has been minimized.

@InExtremaRes
Copy link

Sorry for a new not-so-relevant post but I just wanted to reply to last @fdc-viktor-luft comment.

The specific case you presented is better handled by Pick instead of Partial. I would made a signature more like this:

set<S, K extends keyof S>(state: Pick<S, K>): void;

Now TS will fail if you pass undefined to a prop that doesn't allow it explicitly. See playground. BTW, this is how setState is typed in React.

That being said I really believe that differentiate "missing" from undefined is necessary. Wish to see some progress on this.

@fdc-viktor-luft
Copy link

@InExtremaRes Wow 🤩 Thanks a lot. That really solves my particular problem, but it's not very obvious 😄

I totally agree with you 👍

@kahoot-karl
Copy link

Sharing my workaround:

type MakeOptionalNull<T> = Required<
  {
    [P in keyof T]: undefined extends T[P] ? T[P] | null : T[P];
  }
>;

type MakeNullUndefined<T> = {
  [P in keyof T]: null extends T[P] ? Exclude<T[P] | undefined, null> : T[P];
};

type RequireOptionalKeys<T> = MakeNullUndefined<MakeOptionalNull<T>>;
type A = {
  a?: number;
  b: number;
};

type B = RequireOptionalKeys<A>;

// type B = {
//    a: number | undefined;
//    b: number;
//}

@ahejlsberg
Copy link
Member

This feature is now implemented in #43947.

@MartinJohns
Copy link
Contributor

I didn't expect this feature coming any time soon, especially since it's not even on the roadmap, and then BAM! Anders suddenly drops a feature-bomb on us.. again! ❤️

@DanielRosenwasser DanielRosenwasser added Breaking Change Would introduce errors in existing code and removed Experimentation Needed Someone needs to try this out to see what happens labels Jun 3, 2021
@andrewbranch andrewbranch removed this from Next Meeting in Suggestion Backlog Triage Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Would introduce errors in existing code Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.