Fix destructuring with fallback

[ahejlsberg]

With this PR we properly permit destructurings to specify fallbacks:

function f1(options?: { color: string, width?: number }) {
    let { color, width } = options || {};  // Was error, now ok
}

function f2(options?: [string, number?]) {
    let [str, num] = options || [];  // Was error, now ok
}

Previously we would complain that "Initializer provides no value for this binding element and the binding element has no default value".

Fixes #26235.

[ahejlsberg]

@typescript-bot test this
@typescript-bot run dt

[typescript-bot]

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 0895fd6. You can monitor the build here. It should now contribute to this PR's status checks.

[typescript-bot]

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 0895fd6. You can monitor the build here. It should now contribute to this PR's status checks.

[ajafff]

Will this also work with non-optional properties/elements?

function f1(options?: { color: string, width: number }) {
    let { color, width } = options || {};  // does this result in 'color: string | undefined' and 'width: number | undefined'?
}

function f2(options?: [string, number]) {
    let [str, num] = options || []; 
}

[ahejlsberg]

Will this also work with non-optional properties/elements?

We need a few more tweaks to make that work. We currently consider (options || {}).color to be an invalid property access, but it ought to be allowed since we know we'll read undefined from the object literal.

[ahejlsberg]

@typescript-bot test this
@typescript-bot run dt

[typescript-bot]

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 48d343b. You can monitor the build here. It should now contribute to this PR's status checks.

[typescript-bot]

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 48d343b. You can monitor the build here. It should now contribute to this PR's status checks.

[mihailik]

@ajafff
Will this also work with non-optional properties/elements?

@ahejlsberg
We need a few more tweaks to make that work. We currently consider (options || {}).color to be an invalid property access, but it ought to be allowed since we know we'll read undefined from the object literal.

I don't think that's right?

The protection here is not just for runtime error, but also for typos. As @RyanCavanaugh implied in his comment, the intention is to catch errors like this:

function getColorOption(options: { color: string }) {
  return (options || {}).colour;
}

[mihailik]

To clarify: this change is brilliant and very helpful. Thanks a lot!!

Allowing the same for non-optional fields is what sounds wrong to me.

[ahejlsberg]

...the intention is to catch errors like this

Your example with the misspelled property is still an error (the || {} doesn't affect the type as the left argument is known to always be truthy). But consider this:

function getColorOption(options?: { color: string }) {
  return (options || {}).color;
}

Here we now add undefined to the resulting type because it is known we'll read undefined from a property that doesn't exist in an object literal. And you'll still get an error for a misspelled property.

[ajafff]

@ahejlsberg it's actually possible for typos to sneak into the fallback object literal:

function getColorOption(options?: { color: string }) {
  return (options || {colour: 'red'}).color;
}

Would it make sense to use the LHS type as contextual type for the RHS and do excess property checking? Or would that break all other uses of fallback objects?

[sandersn]

This is a breaking change in prettier. See #31762 for details.