Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better support for 'defaultProps' in JSX #23812

Closed
DanielRosenwasser opened this issue May 1, 2018 · 9 comments
Closed

Better support for 'defaultProps' in JSX #23812

DanielRosenwasser opened this issue May 1, 2018 · 9 comments
Assignees
Labels
Domain: JSX/TSX Relates to the JSX parser and emitter Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented May 1, 2018

This proposal aims to fix problems around React's use of defaultProps, which TypeScript has trouble understanding today.

Background

Today, React has a concept called defaultProps; these are props that an external user of a component may consider optional, but are always present in the component definition.

export class Greeting extends React.Component {
  static defaultProps = {
    name: 'stranger'
  }

  render() {
    return (
      <div>Hello, {this.props.name}</div>
    )
  }
}

let myElement1 = <Greeting name="Daniel!"/>; // <div>Hello, Daniel!</div>
let myElement2 = <Greeting />;               // <div>Hello, stranger</div>

Unfortunately, TypeScript --strictNullChecks users who want to take advantage of this are in a tough spot.
They can either specify these props as optional and assert that defaulted properties are present internally,

export interface GreetingProps {
  name?: string
}

export class Greeting extends React.Component<GreetingProps> {
  static defaultProps = {
    name: 'stranger'
  }

  render() {
    // Notice the non-null assertion!
    //                           | |
    //                           | |
    //                           \ /
    //                            V
    return (
      <div>Hello, {this.props.name!.toUpperCase()}</div>
    )
  }
}

or they can jump through levels of indirection:

export interface GreetingProps {
  name?: string
}

// Notice we are using a 'const' declaration here!
export const Greeting: React.Component<GreetingProps> = class extends React.Component<GreetingProps> {
  static defaultProps = {
    name: 'stranger'
  }

  render() {
    return (
      <div>Hello, {this.props.name!.toUpperCase()}</div>
    )
  }
}

Proposals

Using resolution semantics of the JSX factory

Users can already emulate this behavior with React.createElement: https://tinyurl.com/y9jbptt9

// React-like API

export interface Component<P> {
    props: P;
}

export interface ComponentClass<P, Defaults = void> {
    new(props: P): Component<P>;
    defaultProps: Defaults;
}

export interface Element<P> {
    type: string | ComponentClass<P>;
    props: P;
}

// Some additions

type Defaultize<T, Defaults> =
    & Partial<Pick<T, Extract<keyof T, keyof Defaults>>>
    & Pick<T, Exclude<keyof T, keyof Defaults>>;

declare function createElement<Props, Defaults>(
    type: ComponentClass<Props, Defaults>,
    props: Defaultize<Props, Defaults> | null):
    Element<Props>

// Now comes your code...

class FooComponent {
    props: { hello: string, world: number };
    static defaultProps: { hello: "hello" };
}

// works: 'hello' got defaulted
createElement(FooComponent, { world: 100 });

// error: missing 'world'
createElement(FooComponent, { hello: "hello" });

In theory, just passing the right data to the JSX factory should do the work for us.

However, there are some issues:

  1. The Defaultize type is hard to read.
  2. TypeScript treats the object literal in createElement as an inference site, and when inference fails, users get no completions for props. It's not ideal in the general case, but the goal is to continue to give a good experience in JSX.

Add a new type in the JSX namespace

We could potentially add a new field called ComponentDefaultPropNames in JSX which would either be a union of literal types or possibly an object type whose property names describe the respective names that defaultProps could have:

namespace JSX {
    export type ComponentDefaultPropNames = "defaultProps";
}

When writing out a JSX tag for a component, JSX.ComponentDefaultPropNames is looked up on the type of the tag name to determine which properties should be considered optional.

So for example:

<TagName {...someObject}>

If TagName expects props with the type Props, TypeScript can relate typeof someObject to some combination of Props and (typeof TagName)[JSX.ComponentDefaultPropNames].

Potential issues

Under either proposal, some type is inferred from the type of the component's defaultProps property.
Since React's .d.ts files currently define defaultProps to be a Partial<P>, where P is the type parameter for the type of props, we need to avoid that from making all props optional.

To avoid issues here, we could say that only required properties on defaultProps satisfy a JSX element's type. So for example:

interface MyProps {
  name: string;
  age: number;
}

class MyComponent extends React.Component<MyProps> {
  render() {
    return <div>My name is {this.props.name} and I am {this.props.age} years old.</div>;
  }

  static defaultProps: { name?: string, age: number } = {
    name: "Daniel",
    age: 26,
  }
}

// error: 'name' is still considered required
let x = <MyComponent />

// works! 'age' is optional because it was definitely present in the type of 'defaultProps'.
let y = <MyComponent name="Daniel" />
@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript In Discussion Not yet reached consensus Domain: JSX/TSX Relates to the JSX parser and emitter labels May 1, 2018
@mhegazy mhegazy added this to the TypeScript 3.0 milestone May 3, 2018
@weswigham
Copy link
Member

namespace JSX {
export type ComponentDefaultPropNames = "defaultProps";
}

No. We're better than this now. Instead, for your consideration:

namespace JSX {
    export type LibraryManagedProps<TComponent, TProps> = /*...*/
}

which can be defined as:

namespace JSX {
    export type LibraryManagedProps<TComponent, TProps> = 
        TComponent extends { defaultProps: infer D; propTypes: infer P; } ? Defaultize<TProps & InferedPropTypes<P>, D>
        : TComponent extends { defaultProps: infer D } ?  Defaultize<TProps, D>
        : TComponent extends { propTypes: infer P } ? TProps & InferedPropTypes<P>
        : TProps;
}

where InferedPropTypes is a type the maps from the propTypes shapes to normal types. This could handle defaultProps and propTypes, and is a little more aligned with the current goal of attempting to remove the JSX namespace completely (as these are closer to the mechanics the factory function would need to use). TComponent is the static class or sfc, and TProps is the props taken from that component or SFC (so either the props member or first argument). Technically TProps is derivable from TComponent... but, convenience (also we do this to handle SFC overloads, which are not derivable right now and are the major stopping block from using the factory).

We shouldn't tie functionality to strings or properties anymore - we don't need to (we have generic index types now - the property references were a hack for before we had them). And "Defaultize is ugly" isn't really a great argument against it, because that exact type would need to be hardcoded into the checker to make the property-name thing work (except even uglier because it'd have no alias, so would be represented in declaration emit and quick info with no aliasing and a ton of nested conditionals), except it wouldn't be able to be recoded if/when the JSX library's functionality changes - it would hardcode the compiler to the current version and implementation of react (specifically), which should be an nongoal.

@MOZGIII
Copy link

MOZGIII commented Jun 1, 2018

How are Props defined (or should be defined)? I mean the argument of a generic React.Component<T> type. Are they supposed to define just the external interface of the component (i.e., the props that can or have to be set to on a component), or the internal interface (i.e. the props that are accessible from this.props inside of the component)? Didn't dug deep, but there's a possibility that we can solve default props elegantly by somehow redefining the semantics of that T argument.

@DanielRosenwasser DanielRosenwasser added the Fixed A PR has been merged for this issue label Jun 30, 2018
@mhegazy mhegazy removed the In Discussion Not yet reached consensus label Jul 2, 2018
@Cryrivers
Copy link

Cryrivers commented Jul 13, 2018

One thing I'd like to mention is that it doesn't support generic components yet, which has nothing to do with LibraryManagedAttributes, but because TypeScript doesn't support higher kinded type yet.

So if your component has Props<T>, the generic type T would become undefined or never when it's passed to TProps.

UPDATE: One of error messages

tabs.stories.tsx:71:13 - error TS2322: Type 'string' is not assignable to type 'undefined'.

71             value={store.state.currentSelected}
               ~~~~~

  index.tsx:16:3
    16   value: V;
         ~~~~~

@inomdzhon
Copy link

typescript version 3.0.3
@types/react version 16.4.13

defaultProps don't work with HOCs =\

HOCs

import * as React from 'react';
import { Subtract } from 'utility-types';

// services
import { TimeService } from './TimeService';

export type WithServerTimePropsType = {
  serverTime: number;
};

function withServerTime<T extends WithServerTimePropsType>(
    Child: React.ComponentType<T>,
  ): React.ComponentType<Subtract<T, WithServerTimePropsType>> {
    class WithServerTime extends React.Component<Subtract<T, WithServerTimePropsType>, {}> {
      static displayName: string = `WithServerTime(${Child.displayName || Child.name})`;

      render(): React.ReactNode {
        return <Child serverTime={timeService.serverTime} {...this.props} />;
      }
    }
    return WithServerTime;
  };

Component with defaultProps

type PropsType = {
  className?: string;
  timeFormat: TIME_FORMAT;
};

class CountDown extends React.Component<PropsType> {
  static defaultProps: PropsType = {
    timeFormat: 'hms',
  };

  render(): React.ReactNode { ... }
}

Using

const HOCCountDown = withServerTime(CountDown);

// error TS2322: Type '{}' is not assignable to type 'Pick<PropsType, "className" | "timeFormat">'.
  Property 'timeFormat' is missing in type '{}'.
<HOCCountDown />

However I indicated timeFormat is optional.

@weswigham
Copy link
Member

weswigham commented Sep 4, 2018

@inomdzhon You did not actually declare timeFormat as optional for your HOCCountDown; your HoC function returns React.ComponentType<Subtract<T, WithServerTimePropsType>> - that will not implicitly passthru the specific defaultProps from the input type (React.ComponentType only asserts that the defaults are bounded to the props, not what the defaults actually are). In fact, the component you return will happily override whatever defaults the input component specified with no defaults at all. Now, implementation-wise your code is ok, since the inner component's defaults will pickup what your component missed and you otherwise don't use the props, but your type annotations don't claim this in any way! So your input may have defaults, but your HoC wrapper doesn't care about that or inspect that, and your HoC wrapper doesn't claim to persist the defaults of a component it is passed (just the props)! So your types are claiming to construct a component with the same props as the input component uses, but without the same defaults. You need to derive your output defaultProps member type from the input's defaultProps member type to do that (or, alternatively, pull in the defaulted props from the input type into your component instead of the input props by using the LibraryManagedAttributes implementation from react or an equivalent you write yourself). The source of this complexity is a need to be mindful of the difference between the constructor-arguments-props-type and the component-internal-props-type for a component. The generic parameter is the component-internal type, while the public type is derived from that via react internals (represented in typespace by the LibraryManagedAttributes type constructor).

Although it's probably worth mentioning that even if you alter your component to get that right, you may need to be on nightly for it to work, since we partially fixed a related issue in intersected types a few days ago.

@itsdouges
Copy link

itsdouges commented Sep 30, 2018

@weswigham any chance for a code example of how that could work? :)

I've been playing around with it, I have an example here: https://gist.github.com/madou/0de18176fc30c26767e2705f0f5281ef

Running into a problem of // [ts] JSX element type 'WrappedComponent' does not have any construct or call signatures., though. Is there a more appropriate constraint to put on TComponent?

@inomdzhon
Copy link

@Madou woh, it's work 😲...unless this error 🤔

@maschino
Copy link

maschino commented Oct 3, 2018

@Madou I've encountered the same issue - "solved" it by casting the passed component back to a React.ComponentType. You can see it in action here: withForm.tsx

It works, but it really feels more like an ugly workaround. I'd appreciate it if someone would find a nicer solution!

@inomdzhon
Copy link

inomdzhon commented Oct 24, 2018

@weswigham hi) what you think about @maschino solution for "provide" defaultProps to HOC?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: JSX/TSX Relates to the JSX parser and emitter Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

8 participants