Spreading props in JSX on right hand side of && operator gives incorrect undefined warning

[samtjo]

Bug Report

🔎 Search Terms

Spread, Spreading, &&

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about prop spreading.

⏯ Playground Link

https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAJQKYEMDG8BmUIjgIilQ3wG4AocmATzCTgGFdIA7JFmAeTYAUcwAznAC8cAN7k4cGMBgAbJAC44AmFGAsA5hQC+FcmggtVjZkfZc2IuAAox02Qrg7lTcOY7ckfCIICUIgB84pJwRDAArlAscAA8ABYAjIFiMvJIOrEA9EmBuvo0dKbubBwAKgDuED6C1hJSaQqVEAD8yqrqWvmUhsbwbqwWzdZ2DunDLsWD5VU1AgHCwfVwvSaGJRZec3WhDY5KY01VADSheqHhUTGNSMMAZHdxAx6W9GIAdJ-r069zOnBZPLkc7kJAAD0gsDgABMkJgUBE5P0zKUYM0KFksnAALLUFYQWFwTQQJBCeJIIhAA

💻 Code

import React from "react";

type ComponentOneProps = {
  title: string;
};

const ComponentOne = ({ title }: ComponentOneProps) => {
  return <h1>{title}</h1>;
};

type ComponentTwoProps = {
  titleTwo?: string;
};

const ComponentTwo = ({ titleTwo }: ComponentTwoProps) => {
  const componentOneProps = {
    title: titleTwo,
  };
  return titleTwo && <ComponentOne {...componentOneProps} />;
};

export default ComponentTwo;

🙁 Actual behavior

Because we checking that titleTwo is truthy here (as it's the first operand of the logical AND operator) - the title prop of ComponentOne can never actually be set to undefined, but we get a warning saying type Type 'string | undefined' is not assignable to type 'string'.

🙂 Expected behavior

I would expect no error here, as the title prop can never be actually set to a falsey value on ComponentOne there. I think this may be an issue with spreading the props onto ComponentOne here as it works if you set the title prop directly:

[MartinJohns]

Here's a simplified example of what you're trying to do:

const value = undefined as string | undefined
const obj = { value }
if (value) {
  const expected: string = obj.value
}

Narrowing the value you stored in an object previously will not narrow the object.

[samtjo]

Thanks for the fast reply @MartinJohns!

So in that case, in your example, changing the if statement to if (obj.value) fixes the TypeScript error, but in my example changing ComponentTwo to look like the following gives the same error message:

const ComponentTwo = ({ titleTwo }: ComponentTwoProps) => {
  const componentOneProps = {
    title: titleTwo,
  };
  return componentOneProps.title && <ComponentOne {...componentOneProps} />;
};

That should solve the error from my understanding of your reply, am I missing something? I'm not sure why TypeScript still thinks componentOneProps.title could be passed down to ComponentTwo as undefined 🤔

[MartinJohns]

Narrowing the property componentOneProps.title will not narrow the object componentOneProps to the type { title: string }.

Quoting @jack-williams:

Type guards do not propagate type narrowings to parent objects. The narrowing is only applied upon access of the narrowed property which is why the destructing function works, but the reference function does not. Narrowing the parent would involve synthesizing new types which would be expensive. More detail in this comment.

[samtjo]

Ok @MartinJohns - do you know if support for this will be added into TypeScript? It would be great if it was able to handle this and it seems like it crops up in the issues here quite often.

This comment from @RyanCavanaugh suggests that it may be possible to add support for this, although that was over a year ago now.

[fatcerberus]

As far as I was aware, "not narrowing the parent type based on individual properties" is deliberate for performance/complexity reasons (so as not to create large intersection types, e.g.) and the tradeoff is that TS provides support for discriminated unions as an alternative.

[karlhorky]

"not narrowing the parent type based on individual properties" is deliberate for performance/complexity reasons

@fatcerberus do you have a link for this somewhere? Would be interested in this.

Another thing that surprised me that does not work is this (Playground):

function fn(h: { a: string; b?: never; c?: never } | { a: string; b: string; c: string }) {}

let data = 'a' as 'a' | undefined;

// ✅ This works in TS
fn(data ? { a: '', b: '', c: data } : { a: '' });

// ❌ TS cannot figure this option out, even though it's also valid
fn({
  a: '',
  ...(data && { b: '', c: data }),
});

// ❌ This also fails
fn({
  a: '',
  ...(data ? { b: '', c: data } : {}),
});

// ✅ This works in TS
fn({
  a: '',
  ...(data ? { b: '', c: data } : {} as {[key: string]: never}),
});

// ✅ This also works in TS
fn({
  a: '',
  ...(true && { b: '', c: '' }),
});

[fatcerberus]

@karlhorky See this comment, which in turn links to this comment chain.

[fatcerberus]

In particular this is relevant:

One concern is how this would affect performance. It seems that for every reference to x in a control flow graph we would now have to examine every type guard that has x as a base name in a dotted name. In other words, in order to know the type of x we'd have to look at all type guards for properties of x. That has the potential to generate a lot of work.

in other words, the effective type of any reference is found by working backwards through the control flow graph, and having to narrow at all levels of an object would give the compiler a lot more work.

[karlhorky]

I created a new issue #50922 just in case any of this is actually a bug

[karlhorky]

@RyanCavanaugh I saw you closed this as "completed" - is the original problem described by @samtjo now fixed in the latest nightly / main branch? Is there somewhere we can read about the change?

Or is this rather supposed to be closed as "wontfix"? (eg. maybe because of same reasons from #50922 (comment) ?)

[RyanCavanaugh]

The bulk "Close" action I applied to Design Limitation issues doesn't let you pick what kind of "Close" you're doing, so don't read too much into any issue's particular bit on that. I wish GitHub didn't make this distinction without a way to specify it in many UI actions!

The correct state for Design Limitation issues is closed since we don't have any plausible way of fixing them, and "Open" issues are for representing "work left to be done".