benoitvallon / react-native-nw-react-calculator

Mobile, desktop and website Apps with the same code
MIT License
5.21k stars 865 forks source link

Use ES2015 classes for subcomponents instead of imported render functions #9

Open cjb opened 8 years ago

cjb commented 8 years ago

Hi! I know I recommended the Render.call() thing initially, but we've since discovered that you can just import a native class that extends Component and render that, which feels much more React-y, and allows the native subclass to override any method of Component (such as storing its own state if it wants to) rather than just Render().

Here's a PR that shows how that works. It's totally untested, though, please test carefully if you decide you'd like to merge! No worries if you decide you prefer the old method. I just thought I'd share.

benoitvallon commented 8 years ago

@cjb very very interesting approach.

I am not going to merge it for 2 reasons (at least now). First even if more "react-y" I think it makes the code harder to get for people who are not so used to react and the goal of the repo was more to demonstrate how to use react to make the 3 apps rather than having a high technical demo of React, and second I think it would be nice to have the android app merged and structured with a more classical way of doing javascript first.

About the tests, they all passed and the react-native build too so I think your version works great.

About merging, we will see in the future because this approach is so interesting and I think may also help to get a simpler structure when the android version will be merged because thinking in terms of components imbrication may be easier to understand.

Thanks for your suggestion

cjb commented 8 years ago

Cool, no worries.