search

Limenius / ReactBundle

Client and Server-side React.js rendering in a Symfony Bundle
MIT License
390 stars 53 forks source link

Remove 'direct props' section in readme #10

Closed teameh closed 8 years ago

teameh commented 8 years ago

Sorry, I added this because I thought it was the case but apparently it isn't.. would be nice though..

teameh commented 8 years ago

Maybe this would be easier and still nice: 

 {{ react_component('RecipesApp', props, {'rendering': 'both'}) }}
nacmartin commented 8 years ago

Mmm, I think that it would be handy to treat props separately to the other possible options, but then if you don't pass props (because maybe you are using the redux store), it would become {{ react_component('RecipesApp', null, {'rendering': 'both'}) }} , which I don't like particularly. I find it difficult to fundament my reasoning because is based on aesthetic reasons, very subjective.

nacmartin commented 8 years ago

On the other hand, the Redux Twig function uses props instead of {'props':props}. As it does not accept more options at the moment, I think it can still make sense to have a different syntax in react_component, but it is somewhat incoherent.

Also if the Redux tag is going to have more options in the future, then the incoherence will be blatant, so it may be a good idea to have props treated as {{ react_component('RecipesApp', props) }}, and fix all the incoherences for once now that I think more about it.

teameh commented 8 years ago

Hmm, yeah fair point.

{...} so it may be a good idea to have props treated as {{ react_component('RecipesApp', props) }}, and fix all the incoherences for once now that I think more about it.

And then we go for {{ react_component('RecipesApp', null, {'rendering': 'both'}) }} in case of no props you mean?

nacmartin commented 8 years ago

I guess if we go for {{ react_component('RecipesApp', props) }} then {{ react_component('RecipesApp', null, {'rendering': 'both'}) }} is something that can happen, yes. Anyways, since you can configure rendering bundle-wide, you will only have to do {{ react_component('RecipesApp', null, {'rendering': 'both'}) }} if you are overwriting your defaults for a specific component, which is rare, I think.