Limenius / ReactRenderer

Client and Server-side React rendering from PHP
MIT License
237 stars 37 forks source link

opt-in phpexecjs #34

Closed oleg-andreyev closed 4 years ago

oleg-andreyev commented 5 years ago

Installation of nacmartin/phpexecjs should not be forced because user may want to use external SSR. So phpexecjs should be an opt-in dependency.

nacmartin commented 4 years ago

I understand the reasoning. However, this package, and the bundle, is quite complicated to set up for some folks, especially beginners in the frontend world. Making this option, which is the default renderer of ReactBundle an opt in would mean that out of the box this doesn't work.

I think that the trade off here is to have a very small dependency that doesn't hurt much (and that is not used) VS having newcomers fighting with getting this library running in the first place.

My opinion is for keeping the dependency. Thanks for making this a PR instead of a suggestion, though, much appreciated. (feel free also to continue this discussion, I can be convinced, ofc).