kay-is / react-from-zero

A simple (99% ES2015 less) tutorial for React
https://www.fullstackreact.com/react-from-zero/
GNU General Public License v2.0
4.6k stars 405 forks source link

Not careful about mutation #1

Closed pesterhazy closed 8 years ago

pesterhazy commented 8 years ago

Great tutorial!

Some example could be more careful about mutating state. For example, https://github.com/kay-is/react-from-zero/blob/master/10-example-app.html#L93 updates this.state tasks directly. A better approach would be to clone the array before mutating. (It doesn't matter in this particular example, but a tutorial should follow best practices.)

Similarly, https://github.com/kay-is/react-from-zero/blob/master/10-example-app.html#L93 changes the props directly, which is a big no-no with React.

I may be wrong though. I'm not a React expert, after all I'm reading a tutorial on React.

markerikson commented 8 years ago

Seconded. Technically it works, but definitely not a good practice. (Also, the Lesson 10 link is actually still changing state, not _props.)

kay-is commented 8 years ago

@pesterhazy You're right, but those links both point to the same file/line and it seems to me only the first time it's the right link.

pesterhazy commented 8 years ago

@kay-is, sorry about that. The second link is https://github.com/kay-is/react-from-zero/blob/master/10-example-app.html#L32

function TaskList(props) {
    // Print the first element bold
    props.children[0] = <b key={0}>{props.children[0]}</b>
    return <ul>{props.children}</ul>
}

My understanding is that a component should not mutate its own children in this way.

pesterhazy commented 8 years ago