6 ms·

A React.js case study

153 points by xngzng 12y ago

lobster_johnson 12y ago
> I first tried to do this: [...] ...which actually worked, but generated a React error message about an unmounted component. I suspect that you were doing something wrong that triggered this error. Unmounting components based on state is perfectly idiomatic React, and I do it all the time: {this.state.mode === 'map' && <Map/>} You pay for the added DOM work, of course, but this is usually negligible in my experience, especially in your case. You do have to make sure you follow the component lifecycle so that you don't try to use getDOMNode() or similar with a component that is not currently mounted.
- _RPM 12y ago
  How does React work under the hood. JavaScript would normally throw an error if it found <Map /> in the source code. How does React do this? Is it JS?
  - hcarvalhoalves 12y ago
    This is not React's, but a javascript preprocessor called JSX that compiles to JS (like LESS -> CSS). http://facebook.github.io/react/docs/jsx-in-depth.html http://facebook.github.io/react/docs/jsx-in-depth.html Something like <Map foo={bar}/> would compile to a function call Map({foo: bar}). Behind the scenes, React is not dealing with HTML. You can use React without JSX, but it's useful.
    - _RPM 12y ago
      Is React a client-side framework, or server-side?
      - mikewhy 12y ago
        both; either
      - baddox 12y ago
        It's primarily client-side, but React components output a virtual representation of the DOM, so it's relatively simple to render a page on the server and return HTML.
      - evan_ 12y ago
        Generally you run the JSX pre-processor server-side and then everything else gets run client-side.
      - vertex-four 12y ago
        It's a client-side framework. You'll generally run the JSX preprocessor as part of your website's build process (i.e. before uploading it to a server).
ufo 12y ago
Why do className={this.state.playing ? "hidden" : "showing"} instead of <div> {(this.state.playing ? <Board/> : <Wordform/> )} </div>
- sehr 12y ago
  Performance, I think.
  - deleted 12y ago
    [deleted]
- krawaller 12y ago
  I touch on this in a post - I did try the latter, but got a React warning in the console about unmounted components. That, together with a comment in the official docs, made me go with the show/hide approach. Although as pointed out in the Disqus comment, choosingg what to render is also fine, I probably did something silly stemming from not fully understanding the component life cycle.
- pselbert 12y ago
  It is much simpler to animate between two classes in straight CSS. That's the primary reason I favor the `className` approach.
  - lobster_johnson 12y ago
    React has a solution for that which actually automatically applies class names as components are inserted and removed: http://facebook.github.io/react/docs/animation.html http://facebook.github.io/react/docs/animation.html. It's not perfect, and it's better than doing it manually.
illicium 12y ago
I'm not particularly fond of the way React mixes presentation (returning nodes in render()) and logic. Particularly things like: var classes = _.reduce(["flipped","correct","wrong"],function(m,c){return m+(this.state[c]?c+" ":"");},"",this); seem very inelegant compared to say, Angular's ngClass, which does the same thing in a data-driven way. But, that's not to say Angular is free from template logic either.
- noelwelsh 12y ago
  The classSet addon works nicely for the cases I've run into. http://facebook.github.io/react/docs/class-name-manipulation.html http://facebook.github.io/react/docs/class-name-manipulation...
- matchu 12y ago
  This isn't a React issue; this is a failure to use an abstraction where appropriate. React offers the classSet plugin for exactly this use case. http://facebook.github.io/react/docs/class-name-manipulation.html http://facebook.github.io/react/docs/class-name-manipulation... I think the React authors agree that React classes shouldn't contain excessive logic; if your class does, consider extracting it to somewhere more appropriate.
  - krawaller 12y ago
    Oh, indeed, I've completely missed the classSet plugin! That would indeed make the code more elegant. And fair point about the logic perhaps being too complicated here. Thanx for the pointers!
- deleted 12y ago
  [deleted]
- btown 12y ago
  I find that the ability to trivially make the equivalent of Rails "helper" functions as auxiliary methods on the Component class actually tends to encourage separation of logic - it's the same file so there's no context switch needed. On top of that, since JSX makes ternary statements look much more elegant than bracketed if statements (whose conditions can be a great hiding place for complex logic) it further encourages breaking your logic out into auxiliary functions.
maccard 12y ago
Doesn't work properly for me using FF 31.0 I can see the words on the tiles before clicking them.
- krawaller 12y ago
  Might be I've been sloppy with adding -moz- to some css rules. Will take a look!
  - pkroll 12y ago
    Same, I can see the words (backwards) before clicking a tile. Chrome works fine, except it seems like after a match the first click on a tile doesn't do anything, and I have to click again to make it flip.
    - krawaller 12y ago
      The not doing anything is intentional - only when the pair is resolved and the status row goes back to saying "choose a tile" can you click a new tile. This "feature" really needs to be communicated better though, sorry abouy that.
  - 8ig8 12y ago
    Auto-prefixer might help... https://www.npmjs.org/package/grunt-autoprefixer https://www.npmjs.org/package/grunt-autoprefixer
  - SimeVidas 12y ago
    Ironically, Firefox supports the unprefixed backface-visibility property, but the demo only sets the -webkit- prefixed version.
rdtsc 12y ago
I see some criticism here but I think it is worth pointing out this looks like a person learning to use a new technology and sharing their experience, which is great. Sometimes that means using non-idiomatic constructs or just having some rough edges. Please be considerate.
wingspan 12y ago
Thanks for putting this together, always interesting to see people use React for the first time! I did a code review of sorts and cleaned up the code to be more idiomatic React. You can find the series of commits with in-depth comments on GitHub [1]. I may even write this up as a blog post on my website [2]. In short: - Don't pass around components and call methods on them; prefer to pass props instead - Conditional rendering instead of hiding, exactly as [3] - Use the JSX harmony transforms - Declare props - Use classSet - setState only needs to include props that change, rest stay the same [1] https://github.com/ianobermiller/reactexperiment/commits/ https://github.com/ianobermiller/reactexperiment/commits/ [2] http://ianobermiller.com http://ianobermiller.com [3] https://news.ycombinator.com/item?id=8247422 https://news.ycombinator.com/item?id=8247422
- krawaller 12y ago
  Wow, thank you so much for this! Reading through your changes I do feel a bit silly. And wiser! :)
  - wingspan 12y ago
    Absolutely! It is really hard to write idiomatic code your first time through with a framework. No need to feel silly at all. No way I would have spotted these things if I hadn't been using React every day for the past year or so!
- e1g 12y ago
  >Use the JSX harmony transforms Thank you for flagging the --harmony flag in the JSX compiler. Just now I was testing Google traceur and es6-module-transpiler and did not realise I already have some of that available for free. I couldn't find any specific information on what subset of ES6 is supported the JSX compiler - would you have a link outlining that?
  - andreypopp 12y ago
    https://github.com/facebook/jstransform/tree/master/visitors https://github.com/facebook/jstransform/tree/master/visitors
theycallmemorty 12y ago
Off-topic: Why do people use lodash over underscore.js?
krawaller 12y ago
There's now a follow-up to this post, walking through a refactor based on the comments given here and elsewhere. Thank you all for the feedback! https://news.ycombinator.com/item?id=8273026 https://news.ycombinator.com/item?id=8273026

153 points by xngzng 12y ago

> I first tried to do this: [...] ...which actually worked, but generated a React error message about an unmounted component. I suspect that you were doing something wrong that triggered this error. Unmounting components based on state is perfectly idiomatic React, and I do it all the time: {this.state.mode === 'map' && <Map/>} You pay for the added DOM work, of course, but this is usually negligible in my experience, especially in your case. You do have to make sure you follow the component lifecycle so that you don't try to use getDOMNode() or similar with a component that is not currently mounted.

_RPM 12y ago
How does React work under the hood. JavaScript would normally throw an error if it found <Map /> in the source code. How does React do this? Is it JS?
- hcarvalhoalves 12y ago
  This is not React's, but a javascript preprocessor called JSX that compiles to JS (like LESS -> CSS). http://facebook.github.io/react/docs/jsx-in-depth.html http://facebook.github.io/react/docs/jsx-in-depth.html Something like <Map foo={bar}/> would compile to a function call Map({foo: bar}). Behind the scenes, React is not dealing with HTML. You can use React without JSX, but it's useful.
  - _RPM 12y ago
    Is React a client-side framework, or server-side?
    - mikewhy 12y ago
      both; either
    - baddox 12y ago
      It's primarily client-side, but React components output a virtual representation of the DOM, so it's relatively simple to render a page on the server and return HTML.
    - evan_ 12y ago
      Generally you run the JSX pre-processor server-side and then everything else gets run client-side.
    - vertex-four 12y ago
      It's a client-side framework. You'll generally run the JSX preprocessor as part of your website's build process (i.e. before uploading it to a server).

ufo 12y ago

Why do className={this.state.playing ? "hidden" : "showing"} instead of <div> {(this.state.playing ? <Board/> : <Wordform/> )} </div>

sehr 12y ago
Performance, I think.
- deleted 12y ago
  [deleted]
krawaller 12y ago
I touch on this in a post - I did try the latter, but got a React warning in the console about unmounted components. That, together with a comment in the official docs, made me go with the show/hide approach. Although as pointed out in the Disqus comment, choosingg what to render is also fine, I probably did something silly stemming from not fully understanding the component life cycle.
pselbert 12y ago
It is much simpler to animate between two classes in straight CSS. That's the primary reason I favor the `className` approach.
- lobster_johnson 12y ago
  React has a solution for that which actually automatically applies class names as components are inserted and removed: http://facebook.github.io/react/docs/animation.html http://facebook.github.io/react/docs/animation.html. It's not perfect, and it's better than doing it manually.

illicium 12y ago

I'm not particularly fond of the way React mixes presentation (returning nodes in render()) and logic. Particularly things like: var classes = _.reduce(["flipped","correct","wrong"],function(m,c){return m+(this.state[c]?c+" ":"");},"",this); seem very inelegant compared to say, Angular's ngClass, which does the same thing in a data-driven way. But, that's not to say Angular is free from template logic either.

noelwelsh 12y ago
The classSet addon works nicely for the cases I've run into. http://facebook.github.io/react/docs/class-name-manipulation.html http://facebook.github.io/react/docs/class-name-manipulation...
matchu 12y ago
This isn't a React issue; this is a failure to use an abstraction where appropriate. React offers the classSet plugin for exactly this use case. http://facebook.github.io/react/docs/class-name-manipulation.html http://facebook.github.io/react/docs/class-name-manipulation... I think the React authors agree that React classes shouldn't contain excessive logic; if your class does, consider extracting it to somewhere more appropriate.
- krawaller 12y ago
  Oh, indeed, I've completely missed the classSet plugin! That would indeed make the code more elegant. And fair point about the logic perhaps being too complicated here. Thanx for the pointers!
deleted 12y ago
[deleted]
btown 12y ago
I find that the ability to trivially make the equivalent of Rails "helper" functions as auxiliary methods on the Component class actually tends to encourage separation of logic - it's the same file so there's no context switch needed. On top of that, since JSX makes ternary statements look much more elegant than bracketed if statements (whose conditions can be a great hiding place for complex logic) it further encourages breaking your logic out into auxiliary functions.

maccard 12y ago

Doesn't work properly for me using FF 31.0 I can see the words on the tiles before clicking them.

krawaller 12y ago
Might be I've been sloppy with adding -moz- to some css rules. Will take a look!
- pkroll 12y ago
  Same, I can see the words (backwards) before clicking a tile. Chrome works fine, except it seems like after a match the first click on a tile doesn't do anything, and I have to click again to make it flip.
  - krawaller 12y ago
    The not doing anything is intentional - only when the pair is resolved and the status row goes back to saying "choose a tile" can you click a new tile. This "feature" really needs to be communicated better though, sorry abouy that.
- 8ig8 12y ago
  Auto-prefixer might help... https://www.npmjs.org/package/grunt-autoprefixer https://www.npmjs.org/package/grunt-autoprefixer
- SimeVidas 12y ago
  Ironically, Firefox supports the unprefixed backface-visibility property, but the demo only sets the -webkit- prefixed version.

rdtsc 12y ago

I see some criticism here but I think it is worth pointing out this looks like a person learning to use a new technology and sharing their experience, which is great. Sometimes that means using non-idiomatic constructs or just having some rough edges. Please be considerate.

wingspan 12y ago

Thanks for putting this together, always interesting to see people use React for the first time! I did a code review of sorts and cleaned up the code to be more idiomatic React. You can find the series of commits with in-depth comments on GitHub [1]. I may even write this up as a blog post on my website [2]. In short: - Don't pass around components and call methods on them; prefer to pass props instead - Conditional rendering instead of hiding, exactly as [3] - Use the JSX harmony transforms - Declare props - Use classSet - setState only needs to include props that change, rest stay the same [1] https://github.com/ianobermiller/reactexperiment/commits/ https://github.com/ianobermiller/reactexperiment/commits/ [2] http://ianobermiller.com http://ianobermiller.com [3] https://news.ycombinator.com/item?id=8247422 https://news.ycombinator.com/item?id=8247422

krawaller 12y ago
Wow, thank you so much for this! Reading through your changes I do feel a bit silly. And wiser! :)
- wingspan 12y ago
  Absolutely! It is really hard to write idiomatic code your first time through with a framework. No need to feel silly at all. No way I would have spotted these things if I hadn't been using React every day for the past year or so!
e1g 12y ago
>Use the JSX harmony transforms Thank you for flagging the --harmony flag in the JSX compiler. Just now I was testing Google traceur and es6-module-transpiler and did not realise I already have some of that available for free. I couldn't find any specific information on what subset of ES6 is supported the JSX compiler - would you have a link outlining that?
- andreypopp 12y ago
  https://github.com/facebook/jstransform/tree/master/visitors https://github.com/facebook/jstransform/tree/master/visitors

theycallmemorty 12y ago

Off-topic: Why do people use lodash over underscore.js?

krawaller 12y ago

There's now a follow-up to this post, walking through a refactor based on the comments given here and elsewhere. Thank you all for the feedback! https://news.ycombinator.com/item?id=8273026 https://news.ycombinator.com/item?id=8273026