Anti-patterns Discussion session
Anti-pattern
Anti-patterns are certain patterns in software development that are considered bad programming practices.
The same pattern may have been considered correct at one point in the past, but now developers have realized that they cause more pain and hard-to-track bugs in the long term.
Anti-pattern
- You are probably right and there is no bug in the code, but you should not do it.
Topic for discussion
Discussion
- Use of index as key in component
- Using object literals directly to component
- Copying props to state
- Handling multiple Promises
- Binding Event to component or React element
- Using setState
- Find the issue
Use index as key in component
Text
import React, { Component } from 'react'; import { render } from 'react-dom'; import Hello from './Hello'; import './style.css'; class App extends Component { constructor() { super(); this.state = { list: [ { id: "1", name: "Suraj K.C." }, { id: "2", name: "Ajay Shrestha" }, { id: "3", name: "Sujan Shrestha" }, { id: "4", name: "Arjun Yonjan" }, { id: "5", name: "Saraswati Shrestha" }, { id: "6", name: "Rosy Shrestha" }, { id: "7", name: "Aishwarya Shrestha" } ] }; } update = () => { const list = [...this.state.list]; list.splice(1,1); this.setState({ list }) } render() { return ( <div> <NameList nameList = {this.state.list} /> <button onClick={this.update}>click me</button> </div> ); } } class NameList extends React.Component { render () { return this.props.nameList.map((name, index) => { return <Name key={index} {...name} /> }); } } class Name extends React.Component { componentWillUpdate(nextProps) { console.log(this.props.name, nextProps.name); } render () { return ( <div>Name: {this.props.name}</div> ) } } render(<App />, document.getElementById('root'));
Problem
React use key to match children in the original tree. The keys are used for identification
DEMO
Solution
key should be unique, stable, Predictable
Like index, random numbers or timestamps should not be used as keys
Using object literals directly to component
Demo
Reason & Solution?
Solution
Avoid copying props into state
class EmailInput extends Component { state = { email: this.props.email }; render() { return <input onChange={this.handleChange} value={this.state.email} />; } handleChange = event => { this.setState({ email: event.target.value }); }; componentWillReceiveProps(nextProps) { // This will erase any local state updates! // Do not do this. this.setState({ email: nextProps.email }); } }
Problem
- Adding additional layer to change state won't help
class EmailInput extends Component { state = { email: this.props.email }; componentWillReceiveProps(nextProps) { // Any time props.email changes, update state. if (nextProps.email !== this.props.email) { this.setState({ email: nextProps.email }); } } // ... }
But it is an anti-pattern
Solutions
- Data down action up approach
- Redux
class EmailInput extends Component { render () { return <input value={this.props.email} onChange={this.props.onChange} /> } } class Form extends Component { state = { email:"" } onChange = (event) => { this.setState({ email: event.target.value }) } render() { return ( <EmailInput email={this.state.email} onChange={this.onChange} /> ) } }
Promises
loadSomething().then(function(something) { loadAnotherthing().then(function(another) { DoSomethingOnThem(something, another); }); });
Problem?
Solution
promise.all([loadSomething(), loadAnotherThing()]) .spread(function(something, another) { DoSomethingOnThem(something, another); });
Use of bind in react app
<input onChange={this.updateValue.bind(this)} value={this.state.name} /> <input onChange={ (evt) => this.setState({ name: evt.target.value }) } value={this.state.name} />
Problem?
Solution
- arrow function
- use bind in constructor
class EmailInput extends React.Component { constructor(props) { this.state = { value: "" } this.onChange = this.onChange.bind(this); // creates new instance of function } function onChange(event) { this.setState({value: event.target.value}) } render() { <input value={this.state.value} onChange={this.onChange} /> } } class EmailInput extends React.Component { constructor(props) { this.state = { value: "" } } onChange = (event) => { this.setState({value: event.target.value}) } render() { <input value={this.state.value} onChange={this.onChange} /> } }
Using Set state
class MyComponent extends Component { constructor(props) { super(props); this.state = { counter: 350 }; } updateCounter() { this.state.counter = this.state.counter + this.props.increment; this.setState({ counter: this.state.counter + this.props.increment; }); this.setState({ counter: this.state.counter + this.props.increment; }); } ... }
Problem?
class MyComponent extends Component { constructor(props) { super(props); this.state = { counter: 350 }; } updateCounter() { // this line will not work this.state.counter = this.state.counter + this.props.increment; // --------------------------------- // this will not work as intended // May not render this.setState({ counter: this.state.counter + this.props.increment; }); // what value this.state.counter have? this.setState({ counter: this.state.counter + this.props.increment; }); } ... }
class MyComponent extends Component { constructor(props) { super(props); this.state = { counter: 350 }; } updateCounter() { // this will work this.setState((prevState, props) => ({ counter: prevState.counter + props.increment })); this.setState((prevState, props) => ({ counter: prevState.counter + props.increment })); } ... }
Find the issue?
import React, { Component } from 'react'; import axios from 'axios'; // API import { onLoadUser } from './UserAPI'; class Example extends Component { state = { isLoading: false, user: {}, } componentDidMount() { this.onLoadUser(); } onLoadUser = async () => { try { this.setState({ isLoading: true }); const data = await onLoadUser(this.signal.token); } catch (error) { this.setState({ isLoading: false }); } } render() { return ( <div> <pre>{JSON.stringify(this.state.user, null, 2)}</pre> </div> ) } }
Solution?
import React, { Component } from 'react'; import axios from 'axios'; // API import { onLoadUser } from './UserAPI'; class Example extends Component { signal = axios.CancelToken.source(); _isMounted = true; state = { isLoading: false, user: {}, } componentDidMount() { this.onLoadUser(); } componentWillUnmount() { this._isMounted = false; } onLoadUser = async () => { try { this.setState({ isLoading: true }); const data = await onLoadUser(this.signal.token); if(this._isMounted === true) { this.setState({ user: data, isLoading: true }); } } catch (error) { if(this._isMounted === true) { if (axios.isCancel(err)) { console.log('Error: ', err.message); // => prints: Api is being canceled } else { this.setState({ isLoading: false }); } } } } render() { return ( <div> <pre>{JSON.stringify(this.state.user, null, 2)}</pre> </div> ) } }; }
It is considered as anti-pattern
Solution
- AbortController for fetch
- https://developer.mozilla.org/en-US/docs/Web/API/AbortController
- Cancel Token for axios
- https://gist.github.com/adeelibr/d4d7cc9c1903d77437aef61a1d94ff12
import React, { Component } from 'react'; import axios from 'axios'; // API import { onLoadUser } from './UserAPI'; class Example extends Component { signal = axios.CancelToken.source(); state = { isLoading: false, user: {}, } componentDidMount() { this.onLoadUser(); } componentWillUnmount() { this.signal.cancel('Api is being canceled'); } onLoadUser = async () => { try { this.setState({ isLoading: true }); const data = await onLoadUser(this.signal.token); this.setState({ user: data, isLoading: true }); } catch (error) { if (axios.isCancel(err)) { console.log('Error: ', err.message); // => prints: Api is being canceled } else { this.setState({ isLoading: false }); } } } render() { return ( <div> <pre>{JSON.stringify(this.state.user, null, 2)}</pre> </div> ) } }; }
Anti-patterns Discussion session
deck
By Ajay Shrestha
deck
- 374