zetachang / react.rb

Opal Ruby wrapper of React.js library.
http://reactrb.org/
MIT License
547 stars 35 forks source link

React source in GEM #138

Open wied03 opened 8 years ago

wied03 commented 8 years ago

This is another react-opal->react.rb milestone issue.

I noticed there is a copy of React in sources. Although it can't be that crazy to have React in a React GEM, perhaps it should be removed.

Between rails-assets, NPM, and the copy already inside react-rails (with lots of control over addons/prod/etc.), it's probably best left to the developer to decide how they want to get it. Plus I wouldn't want to keep up with minor React bug fixes in this code if it were me.

I would say at least until the dust settles with the Opal/JS package question.

Thoughts?

catmando commented 8 years ago

yeah I don't know... fundamentally a lot of these problems revolve around a conflict.

On one hand we would like to make react.rb as easy to use as possible, especially if people are learning or doing some trials.

On the other hand of course we want to keep things efficient.

wied03 commented 8 years ago

Understood. I bet though, among the people on these threads, the 3/4/5 of us each do it a different way though and we have our reasons for doing so.

React's differing builds for prod vs. dev (for a good reason) also complicate this.

What if you just had 2 Gemspecs for the same gem and 1 of them was react and the other was react-bare, the main difference being that 1 of them either bundles the source/depends on rails-assets, etc. and the other leaves it up to you?

wied03 commented 8 years ago

In a way it would be nice if Bundler/rubygems let you ignore dependencies, but that probably opens a Pandora's box that's too dangerous.

catmando commented 8 years ago

I think you are right, that having a basic and full gem version solves it simply. Full = basic + other gems.

ajjahn commented 8 years ago

@wied03 @catmando I think having too many flavors of the gem would become cumbersome as well. I think a solution would be to ship with the latest version of React.js that we support by default, but provide a configuration point to allow the user to specify their own version. When React.rb is loaded up by opal into sprockets, just bring along the correct react copy; or if the user wants to load react in an entirely different way, don't load a copy at all.

wied03 commented 8 years ago

@ajjahn - how are you going to configure that? You have to ensure that if the user doesn't want the bundled react, that sprockets doesn't load it (a Ruby condition on the require won't help for sprockets). You could do an opal stub. It might be possible though that you stub out a file the user actually wanted. To avoid that I think you'd need to make the React source filename fairly react.rb specific.

A separate GEM might be easier.

catmando commented 8 years ago

@ajjahn - if its possible to work out how to do this, it would be great. It would solve other configuration issues as well right?

wied03 commented 8 years ago

Perhaps the simplest solution is to have a single GEM, leave the React source in there, but don't require it. Just make the user add the 1 additional require, e.g. require 'react/sources'

ajjahn commented 8 years ago

@wied03 Perhaps I'm sorely misunderstanding sprockets (quite possible in fact). Can you explain why not requiring the source file wouldn't be enough?

wied03 commented 8 years ago

Not requiring it would be enough. I just didn't think of that option until now. It's not exactly the easiest way for the user, but I'd say it's simple enough and it makes it quite simple on the react.rb side.

catmando commented 8 years ago

Can this mechanism be used for all these cases (browser, active support, rails helpers)

Will this still work if the user wants a different version of react.js?

wied03 commented 8 years ago

Yes and yes. If you don't require it inside react.rb's code, then sprockets will not include it. That leaves new users to either blindly include require 'sources/react.js' on your instructions (or some react-rails type generator that will effectively put it in application.js for them) OR supply their own.

ajjahn commented 8 years ago

We could make it easy on the user like so:

# default, give me everything:
require 'reactrb'
# If the user wants -- piece meal, custom react.js, etc:
require 'reactrb/core'
require 'reactrb/packaged_react' # or bring your own.
require 'reactrb/some_pluggable_feature'

@catmando Active Support doesn't apply here. Like I mentioned in #135, parts of AS are dependencies of react.rb itself.

Rails helpers don't apply either, since they aren't relevant on the opal side of things.

As for browser, we discussed removing it as a dependency in #133, and allowing react.rb to add some convince hooks if it exists. That would look like this:

# If the user wants to mount components via Element['.mydiv''] syntax:
require 'opal-browser' 
require 'reactrb'
sollycatprint commented 8 years ago

This issue was moved to reactrb/reactrb#138