Second use of type guard produces different result

[jasonaden]

Bug Report

🔎 Search Terms

react child.props unknown
ts 18046 reading child prop
typescript user defined type guard fails after second time

🕗 Version & Regression Information

Upgrading project to 4.9.4 from 4.2.4.

• This changed between versions 4.7 and 4.8.2. I went through until I found where this changed.

⏯ Playground Link

Playground link with relevant code

💻 Code

import React from 'react';

const childArray = [] as any as React.ReactNode[];

childArray.forEach(child => {
  const children = React.isValidElement(child) && child.props.children;
  // child.props is of type "unknown" error. If we swap this line with
  // the one above it, the error moves to the other line of code. Only
  // the line where we read child.props does it correctly type the value.
  // Multiple reads of child.props on the first line also work fine.
  const propsValue = React.isValidElement(child) && child.props.value;
});

// Wrapping the reads with one call to isValidElement will type correctly
childArray.forEach(child => {
  if (React.isValidElement(child)) {
    // no errors here
    const children = child.props.children;
    const propsValue = child.props.value;
  }
});

childArray.forEach(child => {
  const isValid = React.isValidElement(child);
  if (isValid) {
    const children = child.props.children;
    const propsValue = child.props.value;
  // after reading the isValid value a second time, child.props is 
  // again unknown
  } else if (isValid) {
    const children = child.props.children;
    const propsValue = child.props.value;
  }
});

🙁 Actual behavior

All examples fail after the second read of a type guard on props, whether re-running the function or just reading the variable.

🙂 Expected behavior

Should behave consistently no matter if this is the first or second read of the type guard.

[RyanCavanaugh]

😱

Anyone interested in trying to produce a repro that doesn't need the entire React typings?

[tjjfvi]

Here you go!

[RyanCavanaugh]

Stellar, thank you!

[RyanCavanaugh]

interface ReactElement<P> {
  props: P;
}
interface SomeReactElementWithFoo extends ReactElement<any> {
  foo: string;
}
type ReactNode = ReactElement<any> | SomeReactElementWithFoo;
declare function isValidElement<T>(object: unknown): object is ReactElement<T>;
function test(obj: ReactNode) {
  const p1 = isValidElement(obj) && obj.props.baz;
  const p2 = isValidElement(obj) && obj.props.baz;
}

[typescript-bot]

The change between origin/release-4.7 and origin/release-4.8 occurred at a4507c9.

[RyanCavanaugh]

Root cause is #50044, cc @ahejlsberg

Possible fixes:

• Change the .d.ts, function isValidElement<T = any> to preserve the old behavior with zero candidates. Basically the generic default here and on ReactElement should really match
• Change the .d.ts to add an overload function isValidElement<T = any>(object: ReactNode<T>) so that a zero-candidate inference doesn't occur
• Do something different in choosing what to do in UDTG behavior

[ahejlsberg]

Part of this issue is an effect of #50044. With that PR we consistently favor the asserted type in a type predicate narrowing, meaning that ReactElement<any> narrows to ReactElement<unknown>, which makes a property access like obj.props.baz an error. That is working as intended.

There isn't an error on the first property access is because the ReactNode type includes SomeReactElementWithFoo in a union type that isn't subject to subtype reduction (explicitly written union types aren't subtype reduced an thus may contain redundant constituents). Following the first isValidElement(obj) check, the type of obj is narrowed to ReactElement<unknown> | SomeReactElementWithFoo, and the property access succeeds because SomeReactElementWithFoo is still in the type. But control flow analysis then performs subtype reduction of the two possible control flows and we end up eliminating SomeReactElementWithFoo. Thereafter, the property access becomes an error.

So, overall this is working as intended, though I'll admit it's subtle.

[ahejlsberg]

BTW, I'll second @RyanCavanaugh's suggested fix:

Change the .d.ts, function isValidElement<T = any> to preserve the old behavior with zero candidates. Basically the generic default here and on ReactElement should really match

[jasonaden]

Thanks for checking into this. I'll PR against React library to land a fix.

[RyanCavanaugh]

I'm not quite willing to sign on to "intended" (lol) but given that the UDTG definition is extremely suspect I think we can call this "best we can do for now". Hopefully one day we can solve the any/unknown subtyping dilemma.