Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Thank you for posting my article. It would be awesome to hear all of your thoughts and to applause the article if you liked it :)


Just a side note - you don't need the constructor boilerplate to initialize state in React components if you use property initializers (I see you're already doing it for handleInput): https://babeljs.io/docs/en/babel-plugin-transform-class-prop...

Additionally, you don't need to .bind on deleteItem since functions initialized as class properties are automatically bound (you should just be able to do deleteItem={() => this.deleteItem(key)}).

Another thing is that there are certain weird things that happen if you use the index as the key for ToDo's render function (consider using the actual item text itself): https://medium.com/@robinpokorny/index-as-a-key-is-an-anti-p...

Overall, great article! Hope that's not too nitpicky!


> you should just be able to do deleteItem={() => this.deleteItem(key)}

Don’t do this. Arrow functions (and bind) are reallocated on every render, so this can result in a lot of useless re-renders.


That only matters if the child components are actively attempting to optimize themselves by implementing `shouldComponentUpdate` with a shallow equality check (or are PureComponents, which do the same thing).

Please see this post for clarification on the actual pros and cons of creating functions in `render`: https://cdb.reacttraining.com/react-inline-functions-and-per...


I think you meant allocations instead of rerenders - if you wanted to reduce rerenders, switching to PureComponent would be the way to go. IMO, what you're suggesting is a premature optimization.


An arrow function as a prop is a new function instance on every render causing child components that extend React.PureComponent to render even if no other property has changed.

I consider this a code smell (b/c we had huge problems with it) and I usually give the advice to do it differently. There are exceptions, of course.


Fair enough. In this case, I guess it's a tradeoff between code organization (should the child component be concerned with its position in the parent) versus avoiding rerenders.


I don't think it's a concerns issue. You can simply decide to never create functions in any render(). There's even an eslint rule for it.


The rationale behind that is avoiding re-renders, which can (but doesn't always) improve performance (shouldComponentUpdate can be slower than simply always re-rendering, per a React core developer): https://news.ycombinator.com/item?id=14418576

If you're concerned about allocation, here's what the same core developer has to say about inline functions in render(): https://twitter.com/sophiebits/status/938075351414063104?lan...

Also see the article that acemarke linked above.

This is a textbook case of premature optimization. Why make your code harder to read just to achieve some performance gains that likely aren't even noticeable?

Lastly, regarding the ESLint rule, sophiebits put it better than I can:

I don’t recall ever having seen a credible-looking study about this, including in 2015.


Awesome feedback and many thanks for taking the time to point these things out. The article has started to gain a lot of traction over the past few days and the last thing I want is for there to be parts of the code that are not following best practice (as a result of my relative inexperience with React).


My first thought for the React example would be to replace the CSS files with styled-components, emotion, or one of the other copycat CSS-in-JS libs with an identical base API.

    import { css } from 'emotion';

    const WhateverComponent = () => (
      <div className={css`
        background-color: green;
        height: 100%;
      `} />
    );

    export default WhateverComponent;
...or...

    import styled from 'react-emotion';

    const WhateverComponent = ({ className }) => (
      <div className={className} />
    );

    export default styled(WhateverComponent)`
      background-color: green;
      height: 100%;
    `;
...or...

    import styled from 'react-emotion';

    const BigGreenBox = styled('div')`
      background-color: green;
      height: 100%;
    `

    const WhateverComponent = () => (
      <BigGreenBox />
    );

    export default WhateverComponent;


Hi Sunil, I’m thankful for your article as you helped inform my framework choice for a new project.

Please know this feedback is presented constructively. I had a bit of difficulty following if the example was Vue or React. One place under “How do we mutate data?”, the last word is Vue: followed by React code. Under “How do we pass data through to a child component?” both say “in React”. I’m very uncomfortable giving editorial advice to a stranger this way; it’s a great article and just wanted to trade some polish for the valuable information you shared. Thanks.


that's a great post, very useful and down to earth. Now i think there's room for a "part 2" post, talking about routing, state management for large project, etc.


Can you give a TL;DR? Which one do you prefer and why?


Hi mhermann. I did originally consider writing my preferences and why, but decided against it as my intention was simply to try to objectively highlight the differences when it comes to adding/deleting items, passing props from parent to child, emitting data from child to parent etc - all so that the reader can then form their own opinion.

But seeing as you've asked, I prefer Vue at the moment as you can achieve the same things in React but with fewer lines of code :)


My question as well, I scrolled down to the end and didn't find a result summary?


In a well-written review, the article is the summary.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: