Regression with >= v3 and React, generics, defaultProps

[tkrotoff]

I encountered a regression with TypeScript >= 3 + React, generics and defaultProps

• TypeScript versions: 3.1.0-dev.20180810 and 3.0.1, not with < 3

• Search terms: React defaultProps generics

• Code:

import React from 'react';
import ReactDOM from 'react-dom';

interface BaseProps {
  when?: (value: string) => boolean;
}

interface Props extends BaseProps {
}

class FieldFeedback<P extends Props = BaseProps> extends React.Component<P> {
  static defaultProps = {
    when: () => true
  };

  render() {
    return <div>Hello</div>;
  }
}

// Error: Parameter 'value' implicitly has an 'any' type
const Test = () => <FieldFeedback when={value => console.log(value)} />;

ReactDOM.render(<Test />, document.getElementById('app'));

• Expected behavior: No error

• Actual behavior: Parameter 'value' implicitly has an 'any' type at line when={value => ...}

When removing defaultProps, there is no error:

interface BaseProps {
  when?: (value: string) => boolean;
}

interface Props extends BaseProps {
}

class FieldFeedback<P extends Props = BaseProps> extends React.Component<P> {
  /*static defaultProps = {
    when: () => true
  };*/

  render() {
    return <div>Hello</div>;
  }
}

// No error
const Test = () => <FieldFeedback when={value => console.log(value)} />;

Error vscode (using 3.1.0-dev.20180810) screenshot:

No error vscode (using 3.1.0-dev.20180810) screenshot:

Stumble upon this while upgrading this lib: https://github.com/tkrotoff/react-form-with-constraints to TypeScript 3.

I do not think this is related with @types/react. Using v16.4.9 (latest version) it fails with TypeScript >= 3 and works with TypeScript < 3:

[mattmccutchen]

The "regression" is due to TypeScript 3.0 adding support for the JSX.LibraryManagedAttributes special type defined in @types/react, which is used to adjust the expected type of the JSX attributes based on the defaultProps. The relevant definitions:

class Component<P, S> {
    readonly props: Readonly<{ children?: ReactNode }> & Readonly<P>;
    // ...
}

type LibraryManagedAttributes<C, P> =
    C extends { propTypes: infer T; defaultProps: infer D; }
    ? Defaultize<MergePropTypes<P, PropTypes.InferProps<T>>, D>
    : C extends { propTypes: infer T; }
        ? MergePropTypes<P, PropTypes.InferProps<T>>
        : C extends { defaultProps: infer D; }
            ? Defaultize<P, D>  // <--- This case applies.
            : P;

type Defaultize<P, D> = P extends any
    ? string extends keyof P ? P :
        & Pick<P, Exclude<keyof P, keyof D>>
        & Partial<Pick<P, Extract<keyof P, keyof D>>>
        & Partial<Pick<D, Exclude<keyof D, keyof P>>>
    : never;

The expected type of the attributes is based on the props property of the component class:

JSX.LibraryManagedAttributes<typeof FieldFeedback,
    Readonly<{ children?: ReactNode }> & Readonly<P>>

The important things here are the Readonly<P> and the inner conditional type of Defaultize. I was able to reduce the problem to the following, where the {[K in keyof P]: P[K]} plays the same essential role as the Readonly<P>:

interface Props {
  when: (value: string) => boolean;
}

function bad<P extends Props>(
    attrs: string extends keyof P ? {[K in keyof P]: P[K]} : {[K in keyof P]: P[K]}) {}
function good1<P extends Props>(
    attrs: string extends keyof P ? P : {[K in keyof P]: P[K]}) {}
function good2<P extends Props>(
    attrs: {[K in keyof P]: P[K]}) {}

bad({when: value => false});
good1({when: value => false});
good2({when: value => false});

Before using the type of the attrs parameter as a contextual type, the compiler takes its apparent type. For good2, because the mapped type is not instantiable, its apparent type is itself, and the compiler appears to be able to work out the type of the when member. In the other two cases, the conditional type does not simplify, so it is instantiable, so its apparent type is its constraint, which is the union of the constraints of the two branches. The constraint of a generic mapped type is {}, so in bad, the constraint of the entire conditional type is {} and we're out of luck. In good1, the constraint of the conditional type is P | {}, and the compiler appears to be able to work out the type of the when member from that.

I hope this partial analysis is helpful. I don't have a good enough understanding of the intended purposes of the compiler concepts to go any further in speculating about what would be an appropriate fix.

[weswigham]

I opened #26281 as a partial fix for this when I saw this appear in our user suite btw. It at least makes generic index types simplifiable much more often.

[weswigham]

@mattmccutchen I can confirm that your reduced example now builds correctly in master (and have opened #27083 to add your example as a regression test). Does your original project work alongside our latest nightlies?

[mattmccutchen]

The original project belonged to @tkrotoff...

[weswigham]

ah, doh. @tkrotoff I mean. 😺

[weswigham]

ahhh, looking at @tkrotoff 's example, the full Defaultize still doesn't work; just @mattmccutchen 's simplified example.

[weswigham]

Righto, so another problem with defaultize is any - when we're calculating the conditional type's constraint, within the true branch we replace a subsitution type in the root with its substitute. Since P is first checked agaisnt any, it is substituted with any, when then produces any | (Pick<P, Exclude<keyof P, keyof D>> & Partial<Pick<P, Extract<keyof P, keyof D>>> & Partial<Pick<D, Exclude<keyof D, keyof P>>>) which is, ofc, just any.

Two things. Defaultize should probably be P extends unknown and we should probably have the constraint be the intersection of the type parameter and the substitute for it, not just the substitute.

[weswigham]

Scratch that. Has nothing to do with any of that. Real remaining issue is that the contextual type created by inference isn't being applied to the attributes correctly. Haaaaaaaaaaaaaaaaaah.

[weswigham]

Both of your posts actually seriously helped expedite fixing this, so thanks to y'all. Problems which arise in JSX are often complex with many moving parts within the compiler, so reduced repros really help.

[tkrotoff]

Still having this issue with 3.1.0-dev.20180918 :/

See https://travis-ci.org/tkrotoff/react-form-with-constraints/builds/430311000#L569 Parameter 'value' implicitly has an 'any' type with <FieldFeedback when={value => ...} ...

I couldn't isolate the problem. With a simple example like the one above => no problem. With the real code => problem.

git clone [email protected]:tkrotoff/react-form-with-constraints.git
cd react-form-with-constraints
yarn
yarn build
=> error

2:30am here and I have a love/hate feeling about TypeScript with several regressions/blocking issues:

• With v2.9.2 => I have Import types in d.ts files contain relative paths to node_modules #24599 : https://unpkg.com/[email protected]/lib/Async.d.ts (so my npm package is unusable for TypeScript users)
• With v3.0.3 => I have this issue (Regression with >= v3 and React, generics, defaultProps #26395) (too many hacks to bypass)
• With v3.1.x => still have this issue (Regression with >= v3 and React, generics, defaultProps #26395) + a crash Crash: TypeError: Cannot read property 'kind' of undefined (at isConstraintPosition) #26978 : tkrotoff/react-form-with-constraints@aef555f#diff-ebf3993f62ee43aa89246c6b65fd6cd8R56

+ several other non-blocking issues like #13948 (https://github.com/tkrotoff/react-form-with-constraints/blob/ts-26395/examples/Password/App.tsx#L58 - my code is riddle with FIXMEs, won't be fixed before v3.2.x) or #26193 (https://github.com/tkrotoff/react-form-with-constraints/blob/ts-26395/tsconfig-react-es5.json#L10) and others that I don't know how to qualify like https://github.com/tkrotoff/react-form-with-constraints/blob/ts-26395/packages/react-form-with-constraints-native/src/Native.tsx#L79

All this with a small lib (react-form-with-constraints, core is 1500loc).

[weswigham]

Still having this issue

Interesting. React's types are ridiculously complicated, so it's very possible there's multiple layers of edge cases or bugs working against you here. I'll certainly try to look into it.

I have #24599

Actually directly my fault, sorry. The fix for that (we found it in the nightlies) didn't quite make the 2.9 release cycle and got bumped to 3.0. 😦 It happens sometimes.

a crash #26978

I just opened a PR to fix that, thanks to your comment telling me it existed. Sorry bud, sometimes issues haven't been quite triaged correctly right now, since we just cycled to a new manager here and some stuff may have not been triaged correctly during the transition (we get a lot of issue activity - notifications on this repo are almost impossible to use). That one should definitely have been put into 3.1, which we still haven't made a final build for, so hopefully it'll be fixed in the final 3.1 release.

several other non-blocking issues like #13948

Since we didn't end up merging #21070 we're waiting on something like #26797 to lay the groundwork for making it easier to remove some simplifying assumptions we have in place regarding computed property names of instantiable types and unions. It's certainly a bit inconvenient for now, but we have a lot of concerns to work through for any given feature; so it can take awhile as we design and iterate.

others that I don't know how to qualify like https://github.com/tkrotoff/react-form-with-constraints/blob/ts-26395/packages/react-form-with-constraints-native/src/Native.tsx#L79

You've written

  async resetFields(...inputsOrNames: Array<TextInput | string>);

right above your render function. That's taken as an overload signature for the function below, because it's missing an implementation and not marked abstract. There's a reason the error you're suppressing says "implementation expected to be named resetFields"! You should actually implement it:

  async resetFields(...inputsOrNames: Array<TextInput | string>) {}; // Note the curly braces!

or write it as an optional property if the intent is for it not to be implemented but still be in a concrete (not abstract) class:

  resetFields?: (...inputsOrNames: Array<TextInput | string>) => Promise<void>;

or add an abstract modifier to the class-and-method:

export abstract class FormWithConstraints extends _FormWithConstraints {
  //...
  abstract async resetFields(...inputsOrNames: Array<TextInput | string>): void; // Note I also included a return type annotation. You should probably do that, too.
  // ...
}

[weswigham]

Still having this issue

Interesting. React's types are ridiculously complicated, so it's very possible there's multiple layers of edge cases or bugs working against you here. I'll certainly try to look into it.

Got it. I actually had a version of my original fix that fixed it fully but backed down from it because it didn't seem necessary on second glance (and was a bit inelegant). Turns out it was. #27251 is open to fix.