Destructuring causes error for null/undefined properties

[ghost]

The following code:

interface Foo {
    x: number;
}

interface Bar {
    foo?: Foo;
}

function test( { foo: { x } }: Bar ) {
    alert( x );
}

test( {} ); // TypeError: Cannot read property 'x' of undefined

transpiles to:

function test(_a) {
    var x = _a.foo.x;
    alert(x);
}

instead of the (arguably) preferable:

function test(_a) {
    var x;
    if (_a.foo) x = _a.foo.x;
    alert(x);
}

The transpiled code does not check for the presence of optional property foo in the Bar interface. This leads to an error at invocation.

I can see three possible solutions:

1. Always check for member existence when transpiling destructuring statements.
2. Check for member existence only when an element is optional in the interface or type declaration.
3. New syntax for destructuring that can indicate members should be checked.

E.g.

function test( { foo?: { x } }: Bar ) {}

[RyanCavanaugh]

This is just the ES6 destructuring behavior; we don't use type system data to change the runtime behavior.

That said, it's a bad idea to use a property of an optional property as part of a destructuring, and we should probably consider this an error.

[ghost]

I agree that it should be an error if not handled elsewhere.

Destructuring assignment is a great way to clear out boilerplate code, which makes it easier to focus on domain-specific logic. It feels crippled without support for optional parameters (which are a TypeScript feature, not an ES feature).

I feel like if we extend destructuring with optional annotations - in a manner entirely consistent with conditional function parameters - it greatly enhances the utility and applicability of destructuring.

Strawman:

interface AjaxOptions {
    timeout?: number,
    onComplete?: () => void;
    onSuccess?: () => void;
    onError?: () => void;
}

type AjaxParams = { url: string, method?: string, body?: string, data: string, options?: AjaxOptions }; 

function send( { url, method = 'GET', data?, options?: { timeout, onComplete, onSuccess, onError } }: AjaxParams ) {
    // do stuff
}

The code in the body of the example can now check whether timeout is defined without worrying whether options is defined. This reduces developer effort, makes the code less error prone (especially if refactoring a property into or out of the options object), and clarifies the function by removing boilerplate code.

Basically, use the same ? syntax optional parameters use, but for properties. This cascades downward, so child properties of optional properties are themselves implicitly optional. (Or make it an error to have a non-optional descendant of an optional property.)

Using the destructuring assignment expression itself here instead of the metadata means that you must opt-in to this behavior. It is not inferred from the type definition.

[RyanCavanaugh]

Accepting PRs. Any property destructuring must be non-optional or have a default

[Arnavion]

@RyanCavanaugh Just to be clear, are you accepting PRs to make { foo: { x } }: Bar (where Bar.foo is optional) an error? Or to add support for { foo?: { x } }: Bar with the emit @errorx666 is asking for?

Edit: Never mind, I see from the linked issue that it's the former.

[falsandtru]

I already said about this problem in #6125. This issue is duplicate.

[mhegazy]

we are accepting PRs for flagging destructuring into a optional property as an error. We still do not want to do any type directed emit.

[jods4]

This implies that in cases where I know (because of program context) that the optional property is present in my object, I won't be able to destructure?
Even though that would be valid, working JS?

interface X { a?: { b: number } };
let x: X;

if (x.a) { // or some other domain rule that guarantees me 'a' is defined
  let { a: { b } } = x; // error: "a" is optional ?
}

I don't have any use for such code, just pointing out the possible implications if it becomes an error.

[mhegazy]

a should be optional in this context with #2388

[jods4]

@mhegazy Yes but this was just for illustration.
You could know that an optional member is not null because of some domain-specific knowledge.
In fact that's the whole purpose of x!.

So if we forbid destructing an optional member, should we be able to do this?

let { a!: { b } } = x;

Thinking about it, I guess it makes sense.
TS is about preventing errors and destructuring from null or undefined throws a runtime error so it should be caught by TS.
With the new work on null types and data flow analysis, one should be allowed to destructure an optional member that's proven to be present.
And if the program logic implies it is present I should be able to indicate this to TS with a bang.

[TheTedAdams]

I'd be concerned about throwing a compile error when trying to destructure from an optional property.

A common case for me is destructuring a react components state object. In my state interface I have all properties marked as optional so that I can call this.setState({ onlyOnePropertyOfMany: 'value' }). But I, as the developer, know that when I retrieve the state object all the properties are there, so I like to start my render function with const { onlyOnePropertyOfMany, anotherPropertyWhichIsAnObject: { foo, bar } } = this.state;.

I also have optional properties on interfaces for objects I get from and pass to the API. I know that when I get the object it is fully populated, but when sending a PATCH I want to be able to send just the dirty fields while still having a type check that I'm not including any fields not on the object.

Even with all optional properties these interfaces provide value by making sure I spell my properties right with type checking... but this compile error would break some of my code.

[ghost]

@TheTedAdams I believe setState was the use-case for #4889. If the property is optional, then it can be void, and so an error can (and likely will) arise if used with destructuring without a default value. I would argue that, if the property is truly optional, then you should be forced to specify a default, and if the property is not truly optional, then it shouldn't be marked as such.
See also the suggestion for ! indicating that you have situational knowledge that an optional parameter is not void.

[RyanCavanaugh]

These are good points and lead me to believe we should make this change contingent on #7355. I'll reraise for discussion.

[pauldijou]

I think you also have the problem when destructuring arrays.

const [a, b, c] = ''.split('')

TypeScript will consider all three variables to be string while they should be string | undefined (because, they are totally undefined here)

[blakeembrey]

@pauldijou Pretty sure that's unrelated to this issue (this issue is about nested destructuring that can cause a runtime error). See #9235 for index signatures.

[ghost]

The sample code in this issue correctly has a compile error now. Is this resolved?

[jods4]

@Andy-MS can we have a syntax to indicate something nullable is actually set and so the error should be ignored? See my previous comment #6784 (comment)

[CzBuCHi]

what about doing it same way as c# ?. operator? some random blog with explanation

var property  = some?.nested?.property?.name;
// with is equivallent to:
var property = some == null ? null
    : some.nested == null ? null
        : some.nested.property == null ? null
            : some.nested.property.name;

in ts this would be current syntax

var some: { nested?: { property?: { name: string } } } = ...;
var { nested: { property: { name } } } = some; // name has type string | undefined

transpiled into:

var property = ['nested', 'property', 'name'].reduce((obj, prop) => obj && obj[prop], some);

not sure how to handle multiple nested variables ...

[danielbodart]

What's the current status of this?

[schmaluk]

const [a, b] : string[] = [];
const d: string = b.toUpperCase();

In strict-mode should be invalid. Why only at runtime I get aTypeError: Cannot read property 'toUpperCase' of undefined ?

Thought in strict-mode it is guaranteed that values are not null or undefined implicitly :(((
I think it would be great if this can be fixed,
A proper destructuring should handle all cases recursively like an "empty array" somehow...
Maybe by recursion.

[sroze]

This code will dramatically fail because destructuring an undefined value seems to be allowed:

const f1 = (something: string): { foo: string } | undefined => {
  return something == 'foo' ? { foo: 'bar' } : undefined;
};

const { foo } = f1('bar');

Shouldn't TypeScript throw an error if it's the case? Happy to put my hands in TS' code if we have agreement on this expected behaviour :)

[kitsonk]

@sroze it does error if you have strict null checks on. When you run in sloppy mode, it behaves as expected and does not error.

[prajun7]

Ran into the same problem. Any fixes?