CODEWITHSUNDEEP
reactjsreact-native
knot22
knot22

Reputation: 2768

refactor ReactJS component to remove getDerivedStateFromProps

I have the following React component which is working as desired. After reading about fully controlled components in this blog post I'm wondering if this code could be refactored to make it fully controlled, which could eliminate the use of getDerivedStateFromProps, if I understand correctly. Being more familiar with Vue Native than React Native I'm looking for some guidance on how to go about doing such a refactor.

import React, { Component } from "react";
import { Container, Content, Picker } from "native-base";

export default class DynamicPicker extends Component {
  constructor(props) {
    super(props);

    this.state = {
      selected: this.props.selected
    }
  }

  static getDerivedStateFromProps(nextProps, prevState) {
    if (nextProps.selected !== prevState.selected) {
      return ({ selected: nextProps.selected });
    } else {
      return null;
    }
  }

  onValueChange(value) {
    this.setState({
      selected: value
    });
    this.props.onValueChange(value);
  }

  itemsList = () => {
    return (this.props.items.map( (item, index) => {
      return (<Picker.Item label={item} key={index} value={item} />)
    }));
  }

  render() {
    return (
      <Container>
        <Content>
            <Picker
              // note
              mode="dropdown"
              style={{ borderColor: 'gray', borderWidth: 1, height: 40, margin: 5 }}
              selectedValue={this.state.selected}
              onValueChange={this.onValueChange.bind(this)}
            >
              { this.itemsList() }
            </Picker>
        </Content>
      </Container>
    );
  }
}

Upvotes: 0

Views: 116

Answers (1)

Vlad Moisuc
Vlad Moisuc

Reputation: 256

From what I'm seeing you actually don't need any state here or the method onValueChange.

Why do you want to get a value from props just to set it on the state? Can't you just use the selected value received as props?

Also, can't you just use the method onValueChange received from props? I don't see any need to make another one in this component, because you're setting a new state, but you're going to change the state after that because you call this.props.onValueChange. When any prop changes the component will rerender, therefore getDerivedStateFromProps will be called and it will modify the state.

Long story short, this is how I see this component: 

import React, { Component } from "react";
import { Container, Content, Picker } from "native-base";

export default class DynamicPicker extends Component {      
  itemsList = () => 
  // Implicit return from the arrow function  
  this.props.items.map( (item, index) => {
      return (<Picker.Item label={item} key={index} value={item} />)
    });
  }

  render() {
    return (
      <Container>
        <Content>
            <Picker
              // note
              mode="dropdown"
              style={{ borderColor: 'gray', borderWidth: 1, height: 40, margin: 5 }}
              selectedValue={this.props.selected}
              onValueChange={this.props.onValueChange}
            >
              { this.itemsList() }
            </Picker>
        </Content>
      </Container>
    );
  }
}

Upvotes: 1

Related Questions