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

Default props in ES6 class syntax #3725

Closed
philipwalton opened this issue Apr 22, 2015 · 40 comments
Closed

Default props in ES6 class syntax #3725

philipwalton opened this issue Apr 22, 2015 · 40 comments

Comments

@philipwalton
Copy link

@philipwalton philipwalton commented Apr 22, 2015

The ES6 support announcement says:

the idiomatic way to specify class state is to just use a simple instance property. Likewise getDefaultProps and propTypes are really just properties on the constructor.

This makes a lot of sense to me, but I noticed some small inconsistencies that may be worth rethinking.

When using the original .createClass syntax, the value returned by getDefaultProps seems to be used at other points in the component lifecycle -- not just in the constructor. For example, if I inspect what gets sent to componentWillReceiveProps(props), I can see that the default props are applied.

This doesn't seem to be the case when using the ES6 class syntax, which means I have to duplicate code. Here's an example of what I mean:

class Control extends React.Component {

  constructor(props) {
    props.value = props.value || '';
    super(props);
  }

  // ...

  componentWillReceiveProps(props) {
    props.value = props.value || '';
    // Do something with props...
  }

}

As you can see, I'm duplicating the expression props.value = props.value || ''. If I had more than one default, I'd obviously have a lot more duplication.

When using the .createClass method, I could return {value: ''} from the getDefaultProps method, and this would work, and I'd only have to do it once.

Does it make sense to restore this method to avoid unnecessary duplication? Is there another, more React-like approach that I'm not aware of?

@sophiebits
Copy link
Collaborator

@sophiebits sophiebits commented Apr 22, 2015

The default props should be already merged in when componentWillReceiveProps is called.

@philipwalton
Copy link
Author

@philipwalton philipwalton commented Apr 22, 2015

The default props should be already merged in when componentWillReceiveProps is called.

As far as I can tell, getDefaultProps is never called when using the ES6 class syntax.

@sophiebits
Copy link
Collaborator

@sophiebits sophiebits commented Apr 22, 2015

That's correct. Instead, you write this:

class Control extends React.Component {

  // ...

  componentWillReceiveProps(props) {
    // Do something with props...
  }

}
Control.defaultProps = {value: ''};

Does that work for you?

@philipwalton
Copy link
Author

@philipwalton philipwalton commented Apr 22, 2015

Ahh, sorry, I see that in the documentation now. Thanks!

@ghost
Copy link

@ghost ghost commented Jan 5, 2016

Because I landed here from a Google search, and I prefer the class to encapsulate my code, the default props can be set using computed properties like so:

import React, {Component, PropTypes} from 'react'
class DefaultPropsExample extends Component {
  static defaultProps = {
    ...Component.defaultProps,
    instructions: 'Usage instructions not provided.',
  }
}
@Gopikrishna19
Copy link

@Gopikrishna19 Gopikrishna19 commented Jan 25, 2016

@jhabdas What you have given is not a ES2015 syntax, I guess; Babel throws unexpected token if I use the static props, however, for those guys who work like me in es2015, here's a working stuff.

import React from 'react';
export default class extends React.Component {

  static get defaultProps() {
    return {
      // stuff you want :)
    }
  }

}

update

After installing @zpao 's link I can use static properties ...

@zpao
Copy link
Member

@zpao zpao commented Jan 25, 2016

@Gopikrishna19 you would need to enable another babel plugin that implements the class properties proprosal: http://babeljs.io/docs/plugins/transform-class-properties/

@Gopikrishna19
Copy link

@Gopikrishna19 Gopikrishna19 commented Jan 25, 2016

@zpao Ooh! Another new stuff today :) will try that, thanks a lot!

@ConAntonakos
Copy link

@ConAntonakos ConAntonakos commented Feb 28, 2016

@Gopikrishna19 I actually like the code snippet you posted b/c I can do some pre-processing before initializing the default props. Thanks for mentioning it.

@fxhereng
Copy link

@fxhereng fxhereng commented Apr 13, 2016

This seems to be working and it's more elegant in my opinion:

....

 static defaultProps = {
      //someDefaultProps
  };

  constructor(props, defaultProps) {
    super(props, defaultProps);
  }

....
@gaearon
Copy link
Member

@gaearon gaearon commented Apr 13, 2016

@fxhereng The second argument is reserved for context so I’d avoid overriding it with some custom meaning because this might break in future releases.

@fxhereng
Copy link

@fxhereng fxhereng commented Apr 13, 2016

Ok @gaearon What's the best way to set default props for you?

Thanks,

@timini
Copy link

@timini timini commented Apr 13, 2016

What's the recommended way to to this?

@sophiebits
Copy link
Collaborator

@sophiebits sophiebits commented Apr 13, 2016

If you use the experimental transform http://babeljs.io/docs/plugins/transform-class-properties/, then you can just use static defaultProps = {...};. No need for changes to the constructor. Otherwise, you need to assign outside:

class X extends React.Component {
}
X.defaultProps = {...};
@efernandesng
Copy link

@efernandesng efernandesng commented Apr 21, 2016

Also..

class X extends React.Component {
  props = {
    ...
  }
}
@gaearon
Copy link
Member

@gaearon gaearon commented Apr 21, 2016

@efernandesng This is not a supported pattern. It won’t behave like defaultProps and it mutates this.props which you should not do.

@jeanmichelcote
Copy link

@jeanmichelcote jeanmichelcote commented Jul 11, 2016

Seems like I can't get defaultProps to be instantiated with the parent class so that I may pass those props to another child class within.

Is there a standard es6 way of doing that? All of the methods on here don't work for me.

@gaearon
Copy link
Member

@gaearon gaearon commented Jul 11, 2016

Seems like I can't get defaultProps to be instantiated with the parent class so that I may pass those props to another child class within.

I don’t understand what you mean. Can you show an example?

@jeanmichelcote
Copy link

@jeanmichelcote jeanmichelcote commented Jul 12, 2016

Of course, sorry about that.

Here is a component:

import React, 
{
  Component,
  PropTypes
}                     from 'react';
import { TimerView }  from './TimerView'

class Timer extends Component {
  constructor(props) {
    super(props);
  }

  componentWillReceiveProps(props) {
    console.log('Will receive props')
  }

  render() {
    console.log("Timer loaded")

    return (
      <TimerView {...props} />
    )
  }
}

Timer.propTypes = {
  status: PropTypes.string.isRequired,
};

Timer.defaultProps = {
  status: "This is the Timer",
};

export default Timer;

When I run the webpack-dev-server, I get this error:
Uncaught ReferenceError: props is not defined

There must be something I do wrong...

@keyz
Copy link
Contributor

@keyz keyz commented Jul 12, 2016

props is an unbound variable in your render() method. It should be <TimerView {...this.props} />.

@jeanmichelcote
Copy link

@jeanmichelcote jeanmichelcote commented Jul 12, 2016

Oh my.
I totally missed this one. I guess since I am using stateless components as much as classes, I just got confused on this one.

Thank you sir.

@keyz
Copy link
Contributor

@keyz keyz commented Jul 12, 2016

no problem @jeanmichelcote! it happens :)

@romulof
Copy link

@romulof romulof commented Sep 30, 2016

Accessing other props to define defaultProps is considered an anti-pattern?

class Comp extends from React.Component {
  static propTypes: {
    num: React.PropTypes.number.isRequired,
    action: React.PropTypes.func,
  };

  static defaultProps: {
    action: () => console.log(this.props.num), // "this" is invalid in static context
  };

  render() {
    return (
      <button onClick={this.props.action}>
        {`Log #${this.props.num}`}
      </button>
    );
  }
}
@sbussard
Copy link

@sbussard sbussard commented Oct 27, 2016

@romulof it looks like you're trying to reference an instance variable from a static method. You can't do that because there's no way to tell which this you're talking about. By definition, static things always live outside of the context of an instance.

@sbussard
Copy link

@sbussard sbussard commented Oct 27, 2016

@romulof That's common with all object oriented languages. The fat arrow syntax binds to the context it's defined in. Maybe you could try using

static defaultProps: {
    action: function() {
        console.log(this.props.num), // "this" depends on the context where it is run
    }
};
@romulof
Copy link

@romulof romulof commented Oct 27, 2016

@sbussard: Exactly.

My question is about defaultProps being implemented as a static object.
Maybe allowing it to be a function, to be called by constructor.

@gaearon
Copy link
Member

@gaearon gaearon commented Oct 27, 2016

It is intentionally outside the class so that an optimizing compiler could inline it at the call site.

@jsoneaday
Copy link

@jsoneaday jsoneaday commented Jan 29, 2017

So what is the final answer?

@shaunwallace
Copy link

@shaunwallace shaunwallace commented Feb 22, 2017

@gaearon you've mentioned that

It is intentionally outside the class so that an optimizing compiler could inline it at the call site.

but does that mean it cannot be optimized when using "class instance fields" and/or "class static fields"? I guess I am trying to understand which approach is preferred and why.

@gaearon
Copy link
Member

@gaearon gaearon commented Feb 22, 2017

So what is the final answer?

If you want to stick to ES6, assign it at the bottom:

class MyComponent extends Component { /* ... */ }
MyComponent.defaultProps = { /* ... */ };

If you're using Create React App, or if you're comfortable with experimental syntax and have enabled the class properties transform, you can do this:

class MyComponent extends Component {
  static defaultProps = { /* ... */ };
  /* ... */
}

These approaches are completely equivalent. Note that if you're also using decorators, you might have weird bugs combining these transforms. So I don't recommend using decorators together with class properties.

I hope this helps!

but does that mean it cannot be optimized when using "class instance fields" and/or "class static fields"? I guess I am trying to understand which approach is preferred and why.

There is no difference between assigning at the end and using class properties. Class properties desugar to assignment. Please use REPL on Babel website to check what code compiles to.

@roberttod
Copy link

@roberttod roberttod commented Mar 13, 2017

I am curious about the same issue @romulof talks about. Sure, I can use static defaultProps to define default properties but what if I want to use the context?

Why is this useful?

I have a generic modal component which has title and onSubmit props. Sometimes I want to use the Modal with the same props but from completely different parts of my App. For title this works just fine.

class CreateUserModal extends Modal {
  static defaultProps = { title: 'Create a new user' }
}

However the onSubmit handler, which happens to be a decent chunk of code, requires context because it uses a prop populated from connecting redux. Currently there is no way for me to abstract this code because context can't be used to set the props of the parent class.

What I am trying to do isn't really an anti pattern (or at least I think it isn't) so I wonder if there is another way to do this?

I hoped I would be able to do something like

class CreateUserModal extends Modal {
  constructor (props) {
    super({
      title: 'Create a new user',
      onSubmit: () => {
        // do a load of stuff using props
      }
    })
  }
}

but it doesn't work.

@code4cake
Copy link

@code4cake code4cake commented Sep 22, 2017

@roberttod did you find a solution for your problem? Thanks

@ghost
Copy link

@ghost ghost commented Oct 4, 2017

I want to ask a follow up question,
if I'm using create react app (react-scripts v1.0.12), is it okay to define initial states as

class Foo extends Component {
  static defaultProps = {};
  
  static propTypes = {};
  
  state = {...};
}

what is the difference compare to using constructors to define initial states as:

class Foo extends Component {
  static defaultProps = {};
  
  static propTypes = {};
  
  constructor(props) {
    super(props);
  
    this.state = {...};
  }
}

specifically is there any performance implications in the first approach?

@gaearon
Copy link
Member

@gaearon gaearon commented Oct 4, 2017

It's exactly the same thing, there's no difference.

@majioa
Copy link

@majioa majioa commented Oct 9, 2017

@jhabdas Well, so when the defaultProps is defined as:

static defaultProps = {
}

How to access it from the class methods since:

this.defaultProps

returns undefined

@ConAntonakos
Copy link

@ConAntonakos ConAntonakos commented Oct 9, 2017

@majioa I don't know about that class-level API, but one idea is to cache the default props as a separate object and refer to that. What do you think?

@TrySound
Copy link
Contributor

@TrySound TrySound commented Oct 9, 2017

@majioa

class Foo extends React.Component {
  static defaultProps = { param: 1 };
  render() {
    return Foo.defaultProps.param;
  }
}
@levinit
Copy link

@levinit levinit commented Dec 28, 2017

I meet the same problem, you sholud install babel-plugin-transform-class-properties

npm install -D babel-plugin-transform-class-properties

then add "transform-class-properties" to .babelrc

{
  "presets": ["env", "react"],
  "plugins": ["react-hot-loader/babel", "transform-class-properties"]
}
@Arm7107
Copy link

@Arm7107 Arm7107 commented Jan 8, 2018

static get defaultProps() {
return {
car: "Mercedes"
}

@gaearon
Copy link
Member

@gaearon gaearon commented Jan 8, 2018

@Arm7107 This is inefficient because it creates a new object every time an element is created. I strongly advise not to use this. Either assign a property:

MyComponent.defaultProps = {
  // ...
}

or use experimental static class properties syntax:

class MyComponent extends React.Component {
  static defaultProps = {
    // ...
  };

  // ...
}

(for which you'll need a Babel plugin)

I'll lock this issue to prevent further incorrect suggestions that will show up in Google results.

@facebook facebook locked as resolved and limited conversation to collaborators Jan 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.