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

Implementing defaultProps with ts 2.0 strict null checks #11640

Open
cwmoo740 opened this Issue Sep 30, 2016 · 50 comments

Comments

Projects
None yet
@cwmoo740
Copy link
Contributor

cwmoo740 commented Sep 30, 2016

Default properties don't seem to function well currently with strictNullChecks enabled. For example:

interface TestProps { x?: number}

class Test extends React.Component<TestProps, null> {

    static defaultProps =  {x: 5};

    render() {
        const x: number = this.props.x;
        return <p>{x}</p>;
    }
}

Errors with error TS2322: Type 'number | undefined' is not assignable to type 'number' even though this is guaranteed to work at runtime.

Right now defaultProps and Props seem to be treated as the same type always, but they're actually almost never the same type because optional fields in Props should be overwritten by required values in DefaultProps.

What if there were something like...

class ComponentWithDefaultProps<P, D, S> {
    props: P & D & {children?: React.Children};
}

that is identical to the existing React.Component typing except for the type of props?

@JoshuaToenyes

This comment has been minimized.

Copy link

JoshuaToenyes commented Nov 12, 2016

Since the default props are set at runtime, I'm not sure if there's a way to handle this nicely, other than a type assertion. (Of course, you could always just disable strict null checks.)

Here's how you may be able to get around it in your example:

interface TestProps { x?: number}

class Test extends React.Component<TestProps, null> {

    static defaultProps =  {x: 5};

    render() {
        const x: number = (this.props.x as number);
        return <p>{x}</p>;
    }
}

See https://www.typescriptlang.org/docs/handbook/basic-types.html#type-assertions

If there's a more graceful way of handling this, I'd love to hear it.

Disclaimer: I've been using TypeScript for about three days, I'm tired, and probably have no idea what I'm talking about.

@rexk

This comment has been minimized.

Copy link

rexk commented Nov 29, 2016

+1000 on updating type definition file to include three generic type definitions.

This used to be fine without ---strictNullChecks, but now, it is definitely going to be a problem for many component classes.

Flow also implements similar class implementation because of nature of strict null type checking.
https://github.com/facebook/flow/blob/master/lib/react.js#L16
https://github.com/facebook/flow/blob/master/lib/react.js#L104-L105

@pablobirukov

This comment has been minimized.

Copy link
Contributor

pablobirukov commented Feb 13, 2017

Seems like we don't have much options here except of waiting for Microsoft/TypeScript#2175 being resolved in order to add third generic.
I don't think that such a (breaking) change (I mean class Component<P, S, D>) can be approved by reviewers.
@johnnyreilly @bbenezech @pzavolinsky do you guys have an opinion on that?

@pzavolinsky

This comment has been minimized.

Copy link
Contributor

pzavolinsky commented Feb 13, 2017

@R00GER agreed. Changing the definition is too disruptive.

Has anyone considered using Partial?

As in:

    interface ComponentClass<P> {
-        defaultProps?: P;
+        defaultProps?: Partial<P>;
    }
@pzavolinsky

This comment has been minimized.

Copy link
Contributor

pzavolinsky commented Feb 13, 2017

Don't mind that Partial stuff above.

@paranoidjk paranoidjk referenced this issue Apr 7, 2017

Merged

Upgrade tslint #1051

4 of 4 tasks complete

paranoidjk added a commit to ant-design/ant-design-mobile that referenced this issue Apr 7, 2017

paranoidjk added a commit to ant-design/ant-design-mobile that referenced this issue Apr 7, 2017

paranoidjk added a commit to ant-design/ant-design-mobile that referenced this issue Apr 7, 2017

paranoidjk added a commit to ant-design/ant-design-mobile that referenced this issue Apr 8, 2017

paranoidjk added a commit to ant-design/ant-design-mobile that referenced this issue Apr 8, 2017

Upgrade tslint (#1051)
* upgrade antd-tools for new tslint

* Auto fix tslint errors

* fix: tslint

* fix: strictNullCheck with defaultProps

DefinitelyTyped/DefinitelyTyped#11640

* fix: tslint-fix
@nishp1

This comment has been minimized.

Copy link

nishp1 commented Apr 18, 2017

Partial only solves how to declare partial propTypes problem. Inside of render, lastName is still of type string | undefined. To get around it, you have to cast it string using as or ! like shown below. It works, but is not ideal.

interface IUser {
    firstName: string
    lastName?: string
}
export class User extends React.Component<IUser, {}> {
    public static defaultProps: Partial<IUser> = {
        lastName: 'None',
    }

    public render () {
        const { firstName, lastName } = this.props
        // error
        lastName.toUpperCase()

        return (
            <div>{firstName} {lastName}</div>
        )
    }
}

I've just started using TS. Am I missing anything?

@vsaarinen

This comment has been minimized.

Copy link
Contributor

vsaarinen commented Apr 19, 2017

If anyone has a good solution for types and defaultProps, I'm all ears. We currently do this:

interface Props {
  firstName: string;
  lastName?: string;
}

interface DefaultProps {
  lastName: string;
}

type PropsWithDefaults = Props & DefaultProps;

export class User extends React.Component<Props> {
  public static defaultProps: DefaultProps = {
    lastName: 'None',
  }

  public render () {
    const { firstName, lastName } = this.props as PropsWithDefaults;
    
    return (
      <div>{firstName} {lastName}</div>
    )
  }
}
@Glinkis

This comment has been minimized.

Copy link
Contributor

Glinkis commented May 9, 2017

+1
I am currently battling this issue.

@iReallyHateUsernames

This comment has been minimized.

Copy link

iReallyHateUsernames commented Jul 5, 2017

+1

@psivanov

This comment has been minimized.

Copy link

psivanov commented Jul 14, 2017

+1

@aldendaniels

This comment has been minimized.

Copy link

aldendaniels commented Aug 1, 2017

In addition to adding the third type parameter, you'll need the ability to diff props against the defaulted props. Happily as of TS 2.4 this is now possible! See Microsoft/TypeScript#12215 (comment)

@Hotell

This comment has been minimized.

Copy link
Contributor

Hotell commented Aug 17, 2017

IMHO adding third parameter is a big no no, also Flow team knew that and recently they changed that for greater good. It should be type-checker responsibility to know how to handle these kind of things.

Don't get me wrong, I love Typescript, but since Flow 0.53 I have to say it's superior for React development https://medium.com/flow-type/even-better-support-for-react-in-flow-25b0a3485627

@aldendaniels

This comment has been minimized.

Copy link

aldendaniels commented Aug 19, 2017

@Hotell Flow does have three type parameters for React.Component - per the Medium article you linked to Flow can infer class type parameters from the subclass annotations - a neat language-level feature TS doesn't support, but not a type-declaration consideration AFAIK.

@Hotell

This comment has been minimized.

Copy link
Contributor

Hotell commented Aug 20, 2017

@aldendaniels

Flow does have three type parameters for React.Component

nope, it used to be that way before 0.53, not anymore :) facebook/flow@20a5d7d#diff-5ca8a047db3f6ee8d65a46bba4471236R29

@aldendaniels

This comment has been minimized.

Copy link

aldendaniels commented Aug 20, 2017

@Hotell Ah, sure enough! Thanks for correcting me.

AFAIK there's no way in TS to infer the type of the default props though. Using the three-type-parameter approach we'd likely be able to get correct typing without blocking on upstream changes from the TypeScript team.

Do you know of a way to use the inferred type of a static property without passing typeof MyComponent.defaultProps as a type param?

@Grmiade

This comment has been minimized.

Copy link
Contributor

Grmiade commented Sep 5, 2017

Any news on this subject? Does someone do a PR to add a third type parameter and use Microsoft/TypeScript#12215 (comment)?

@kgaregin

This comment has been minimized.

Copy link

kgaregin commented Oct 16, 2017

Upvoting issue: same problem

@marcel-ernst

This comment has been minimized.

Copy link

marcel-ernst commented Nov 16, 2017

+1

@joostlubach

This comment has been minimized.

Copy link

joostlubach commented Nov 18, 2017

I also ran into this, and I've chosen (until this is properly fixed) to abstain from using static defaultProps and instead use a helper HOC:

File components/helpers/withDefaults.tsx:

import * as React from 'react'

export interface ComponentDefaulter<DP> {
  <P extends {[key in keyof DP]?: any}>(Component: React.ComponentType<P>): React.ComponentType<
    Omit<P, keyof DP> &         // Mandate all properties in P and not in DP
    Partial<Pick<P, keyof DP>>  // Accept all properties from P that are in DP, but use type from P
  >
}

export default function withDefaults<DP>(defaultProps: DP): ComponentDefaulter<DP> {
  return Component => props => <Component {...defaultProps} {...props}/>
}

Now I can use:

File components/Button.tsx:

import * as React from 'react'
import withDefaults from './helpers/withDefaults'

export interface ButtonProps {
  label: string
  onPress: () => any
}

export const defaultProps = {
  onPress: () => undefined
}

class Button extends React.Component<ButtonProps> {
  // ...
}

export default withDefaults(defaultProps)(Button)

Three potential downsides (that I can think of):

  1. It requires an HOC, but since this is a quite common paradigm in React world, that seems OK.
  2. You have to declare the props as a generic type parameter, and cannot rely on inference from the props property.
  3. There is no implicit checking of the types of defaultProps, but this can be remedied by specifying export const defaultProps: Partial<ButtonProps> = {...}.
@qiu8310

This comment has been minimized.

Copy link

qiu8310 commented Dec 8, 2017

According to @vsaarinen, I write a base class with props: Props & DefaultProps, so the whole class that extends the base class can directly use this.props without use this.props as PropsWithDefaults.

Like this:

import * as React from 'react'

export class Component<P = {}, S = {}, DP = {}> extends React.Component<P, S> {
  props: Readonly<{ children?: React.ReactNode }> & Readonly<P> & Readonly<DP>
}

export interface Props {
  firstName: string
  lastName?: string
}

export interface DefaultProps {
  lastName: string
}

export class User extends Component<Props, any, DefaultProps> {
  render() {
    const { firstName, lastName } = this.props
    
    // no error
    return (
      <div>{firstName} {lastName.toUpperCase()}</div>
    )
  }
}
@r-tock

This comment has been minimized.

Copy link

r-tock commented Dec 20, 2017

Actually @qiu8310 that didnt work fully, Still had issues with call sites screaming about those default props not being optional. Got it to work with a minor adjustment

import * as React from 'react'

export class Component<P = {}, S = {}, DP = {}> extends React.Component<P, S> {
  // Cast the props as something where readonly fields are non optional
  props = this.props as Readonly<{ children?: React.ReactNode }> & Readonly<P> & Readonly<DP>
}

export interface Props {
  firstName: string
  lastName?: string
}

export interface DefaultProps {
  lastName: string
}

export class User extends Component<Props, any, DefaultProps> {
  render() {
    const { firstName, lastName } = this.props
    
    // no error
    return (
      <div>{firstName} {lastName.toUpperCase()}</div>
    )
  }
}
@Aurelain

This comment has been minimized.

Copy link

Aurelain commented Mar 11, 2018

The following is a variation on the technique mentioned by @R00GER:

interface IUser {
    name: string;
}
const User = class extends React.Component<IUser> {
    public static defaultProps: IUser = {name: "Foo"}
    public render() {
        return <div>{this.props.name}</div>;
    }
} as React.ComponentClass<Partial<IUser>>;
React.createElement(User, {}); // no error, will output "<div>Foo</div>"

Using the above snippet will work, but you will lose the ability to use static properties on User, since it becomes an anonymous class. A hacky solution would be to shadow the class name, like so:

// tslint:disable-next-line:no-shadowed-variable
const User = class User extends React.Component<IUser>

You may now use private static fields inside the class. Public statics are still unusable. Also, notice the need to silence tslint.

@aldendaniels

This comment has been minimized.

Copy link

aldendaniels commented Mar 12, 2018

I thought it worth mentioning that as of TS 2.8, the Exclude type is officially supported:

type Omit<T, K> = Pick<T, Exclude<keyof T, K>>;

See Microsoft/TypeScript#21847.

So all we need is for React.createElement() to require the following in lieu of Props:

Omit<Props, keyof DefaultProps>

The only problem is that in the React declarations, there's no DefaultProps type - for this we need either a third type parameter OR the ability to infer the type of static members as a language feature.

In the meantime, we've been rolling with the following:

/**
 * The Create type allow components to implement a strongly thed create() function
 * that alows the caller to omit props with defaults even though the component expects
 * all props to be populated. The TypeScript React typings do not natively support these.
 */
export type Create<C extends BaseComponent<any, any>, D extends {} = {}> = (
  props?: typeHelpers.ObjectDiff<C['props'], D> & React.ClassAttributes<C>,
  ...children: React.ReactNode[]
) => React.ComponentElement<any, any>;

export interface DomPropsType {
  domProps?: domProps.DomProps;
}

export class BaseComponent<P, S = {}> extends React.Component<P & DomPropsType, S> {
  static create(props?: object, ...children: React.ReactNode[]) {
    return React.createElement(this, props, ...children);
  }

  constructor(props: P & DomPropsType, context?: any) {
  ...
}

And all our components look like:

export class InsertObjectMenu extends BaseComponent<Props, State> {
  static create: Create<InsertObjectMenu, typeof InsertObjectMenu.defaultProps>;
  static defaultProps = {
    promptForImageUpload: true,
  };
  ...
}

Finally we have a lint rule enforcing that the create attribute is declared on all components. We don't use JSX, so we use:

InsertObjectMenu.create({...})

Instead of React.createElement().

We've been using this approach across a large codebase for close to a year now with good success, but we'd love to adopt JSX and this is what's holding us back.

@Hotell

This comment has been minimized.

Copy link
Contributor

Hotell commented Mar 12, 2018

So much time invested in this "simple issue" . I'll just leave this here https://medium.com/@martin_hotell/ultimate-react-component-patterns-with-typescript-2-8-82990c516935 🖖

@goloveychuk

This comment has been minimized.

Copy link

goloveychuk commented Mar 16, 2018

    interface Component<P = {}, S = {}, DP extends Partial<P>=P> extends ComponentLifecycle<P, S> { }
    class Component<P, S, DP extends Partial<P> = P> {
        constructor(props: P & DP, context?: any);

        // We MUST keep setState() as a unified signature because it allows proper checking of the method return type.
        // See: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/18365#issuecomment-351013257
        // Also, the ` | S` allows intellisense to not be dumbisense
        setState<K extends keyof S>(
            state: ((prevState: Readonly<S>, props: P) => (Pick<S, K> | S | null)) | (Pick<S, K> | S | null),
            callback?: () => void
        ): void;

        forceUpdate(callBack?: () => void): void;
        render(): ReactNode;

        // React.Props<T> is now deprecated, which means that the `children`
        // property is not available on `P` by default, even though you can
        // always pass children as variadic arguments to `createElement`.
        // In the future, if we can define its call signature conditionally
        // on the existence of `children` in `P`, then we should  remove this.
        private __externalProps: Readonly<{ children?: ReactNode }> & Readonly<P>;
        props: Readonly<{ children?: ReactNode }> & Readonly<P> & DP;
        state: Readonly<S>;
        context: any;
        refs: {
            [key: string]: ReactInstance
        };
    }

    class PureComponent<P = {}, S = {}, DP extends Partial<P>=P> extends Component<P, S, P> { }


interface ElementAttributesProperty { __externalProps: {}; }

Look carefully at last line.

With this changes we could have

interface Props {
    a: string
    b?: string
    c?: string
}

class Comp extends React.Component<Props, {}, typeof Comp.defaultProps> {
    static defaultProps = {
        b: ''
    }

    render() {
        const {a, b, c} = this.props

        let res = a.concat(b)  // ok
        let res1 = a.concat(c) //fail

        return null
    }
}



const res1= <Comp a=''/> // ok
const res3 = <Comp /> // fail

Which is best we can get if using static defaultProps (ts checker should be changed if we want to omit typeof Comp.defaultProps).
Other options, was already said - HOC, type casts.

@decademoon

This comment has been minimized.

Copy link

decademoon commented Mar 25, 2018

Here's my (very ugly) attempt based on the idea from https://medium.com/@martin_hotell/ultimate-react-component-patterns-with-typescript-2-8-82990c516935:

type ExtractProps<T> = T extends React.ComponentType<infer Q> ? Q : never;
type ExtractDefaultProps<T> = T extends { defaultProps?: infer Q } ? Q : never;
type RequiredProps<P, DP> = Pick<P, Exclude<keyof P, keyof DP>>;
type RequiredAndPartialDefaultProps<RP, DP> = Required<RP> & Partial<DP>;

type ComponentTypeWithDefaultProps<T> =
  React.ComponentType<
    RequiredAndPartialDefaultProps<
      RequiredProps<ExtractProps<T>, ExtractDefaultProps<T>>,
      ExtractDefaultProps<T>
    >
  >;

function withDefaultProps<T extends React.ComponentType<any>>(Comp: T) {
  return Comp as ComponentTypeWithDefaultProps<T>;
}
interface IProps {
  required: number;
  defaulted: number;
}

class Foo extends React.Component<IProps> {
  public static defaultProps = {
    defaulted: 0,
  };
}

// Whichever way you prefer... The former does not require a function call
const FooWithDefaultProps = Foo as ComponentTypeWithDefaultProps<typeof Foo>;
const FooWithDefaultProps = withDefaultProps(Foo);

const f1 = <FooWithDefaultProps />;  // error: missing 'required' prop
const f2 = <FooWithDefaultProps defaulted={0} />;  // error: missing 'required' prop
const f3 = <FooWithDefaultProps required={0} />;  // ok
const f4 = <FooWithDefaultProps required={0} defaulted={0} />;  // ok
@MOZGIII

This comment has been minimized.

Copy link

MOZGIII commented Apr 3, 2018

@decademoon, seems like we could just use this solution at @types/react, can we? I mean, if we replace the usual React.ComponentType with your solution.
If that so, maybe you can create a PR?

@RobRendell

This comment has been minimized.

Copy link

RobRendell commented Apr 23, 2018

@decademoon your definition doesn't handle the case where the non-default props actually include optional fields, i.e.

interface IProps {
  required: number;
  notRequired?: () => void;
  defaulted: number;
}

class Foo extends React.Component<IProps> {
  public static defaultProps = {
    defaulted: 0,
  };
}

I got it working in my case by changing your RequiredAndPartialDefaultProps type to not wrap the "RP" with "Required"

type RequiredAndPartialDefaultProps<RP, DP> = RP & Partial<DP>;

@Schlesiger

This comment has been minimized.

Copy link
Contributor

Schlesiger commented May 8, 2018

I'm surprised there still isn't a proper solution or at least a working HOC on NPM; unless I have missed something.

@goodmind

This comment has been minimized.

Copy link
Contributor

goodmind commented May 9, 2018

@toiletpatrol

This comment has been minimized.

Copy link

toiletpatrol commented May 31, 2018

Hi everyone. Just wanted to say and if you are still reading this thread: I think @JoshuaToenyes made the most meaningful and helpful explanation. This is definitely not an issue so there's nothing to do with it. Use type assertion in this case.

@RobRendell

This comment has been minimized.

Copy link

RobRendell commented Jun 1, 2018

@toiletpatrol actually, @decademoon's solution (with my slight amendment) automatically handles default props just fine. It definitely could be merged into the DT definitions for React to provide the feature standard to everyone.

@vkrol

This comment has been minimized.

Copy link
Contributor

vkrol commented Jun 1, 2018

@RobRendell

This comment has been minimized.

Copy link

RobRendell commented Jun 1, 2018

@vkrol I did see that, but I can drop decademoon's implementation in my codebase right now without waiting for releases of new features.

@MOZGIII

This comment has been minimized.

Copy link

MOZGIII commented Jun 20, 2018

Another workaround that I'm using for now for tricky cases:

const restWithDefaults = { ...Component.defaultProps, ...rest };
return <Component {...restWithDefaults} />;

Nothing wrong with it I guess, so I'm leaving it here as a dirty yet simple workaround.

@Hotell

This comment has been minimized.

Copy link
Contributor

Hotell commented Nov 20, 2018

TS 3.2 and react 16.7 typings are fixing this. can we close ?

gowda added a commit to gowda/todos-react that referenced this issue Nov 24, 2018

Add strict mode to tsconfig
Handling 'possibly undefined or null' type error for
component's defaultProps:

DefinitelyTyped/DefinitelyTyped#11640 (comment)
@feimosi

This comment has been minimized.

Copy link

feimosi commented Dec 6, 2018

@Hotell how should it be handled eventually? I still can't get this working properly

@cbergmiller

This comment has been minimized.

Copy link

cbergmiller commented Feb 5, 2019

To save others some time, here is a link to the release notes of Typescript 3:
Support for defaultProps in JSX

@pronebird

This comment has been minimized.

Copy link
Contributor

pronebird commented Feb 5, 2019

@cbergmiller I am afraid those are the release notes for TypeScript 3.1 🙃

@denieler

This comment has been minimized.

Copy link

denieler commented Feb 26, 2019

still having the same issue with React.FunctionComponent

@mgol

This comment has been minimized.

Copy link

mgol commented Feb 26, 2019

@denieler I wouldn't advise using defaultProps with React.FunctionComponent, it's not natural. It's better to use default function parameters:

interface HelloProps {
  name?: string;
  surname?: string;
}

const HelloComponent: React.FunctionComponent<HelloProps> = ({
  name = 'John',
  surname = 'Smith',
}) => {
  return <div>Hello, {name} {surname}!</div>
};
@glecetre

This comment has been minimized.

Copy link

glecetre commented Mar 6, 2019

@mgol How would you define default function parameters if I didn't want to destructure the props?
I can only think of destructuring only the "defaulted" properties like so:

interface HelloProps {
  name?: string;
  surname?: string;
}

const HelloComponent: React.FunctionComponent<HelloProps> = ({
  name = 'John',
  surname = 'Smith',
  ...props
}) => {
  return <div>Hello, {name} {surname}! You are {props.age} years old.</div>
};

But I find it disgraceful to extract only some of the props.

@Glinkis

This comment has been minimized.

Copy link
Contributor

Glinkis commented Mar 6, 2019

@glecetre You can use:

HelloComponent.defaultProps = {
    name: 'John',
    surname: 'Smith'
}
@vkrol

This comment has been minimized.

@mgol

This comment has been minimized.

Copy link

mgol commented Mar 6, 2019

@glecetre Why do you not want to destructure all the props? It's simpler than defining defaultProps & easier to type. Class-based component's props type may bite you if you export to use externally as props that are required may not be required anymore if there's an entry for them in defaultProps. Using defaultProps also seems a little magical while in parameter destructuring it's all JavaScript.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.