Merge remote-tracking branch 'upstream/master' into generators

Author: xionon

README.md

@@ -23,7 +23,7 @@ As with all gem dependencies, we strongly recommend adding `react-rails` to your
 # If you missed a warning at the top of this README - this is still in development
 # which means this version is not pushed to rubygems.org
 
-gem 'react-rails', '~> 1.0.0'
+gem 'react-rails', '~> 1.0.0.pre', github: 'reactjs/react-rails'
 ```
 
 ## Usage
@@ -35,7 +35,7 @@ In order to use React client-side in your application, you must make sure the br
 You can `require` it in your manifest:
 
 ```js
-// app/assets/application.js
+// app/assets/javascripts/application.js
 
 //= require react
 ```
@@ -69,15 +69,15 @@ Component = React.createClass
 
 ```erb
 <!-- react_ujs will execute `React.renderComponent(HelloMessage({name:"Bob"}), element)` -->
-<div data-react-class="HelloMessage" data-react-props="<%= {:name => 'Bob'}.to_json %>" />
+<div data-react-class="HelloMessage" data-react-props="<%= {name: 'Bob'}.to_json %>" />
 ```
 
 `react_ujs` will also scan DOM elements and call `React.unmountComponentAtNode` on page unload. If you want to disable this behavior, remove `data-react-class` attribute in `componentDidMount`.
 
 To use `react_ujs`, simply `require` it after `react` (and after `turbolinks` if [Turbolinks](https://github.com/rails/turbolinks) is used):
 
 ```js
-// app/assets/application.js
+// app/assets/javascripts/application.js
 
 //= require turbolinks
 //= require react
@@ -89,17 +89,17 @@ To use `react_ujs`, simply `require` it after `react` (and after `turbolinks` if
 There is a view helper method `react_component`. It is designed to work with `react_ujs` and takes a React class name, properties, and HTML options as arguments:
 
 ```ruby
-react_component('HelloMessage', :name => 'John')
+react_component('HelloMessage', name: 'John')
 # <div data-react-class="HelloMessage" data-react-props="{&quot;name&quot;:&quot;John&quot;}"></div>
 ```
 
 By default, a `<div>` element is used. Other tag and HTML attributes can be specified:
 
 ```ruby
-react_component('HelloMessage', {:name => 'John'}, :span)
+react_component('HelloMessage', {name: 'John'}, :span)
 # <span data-...></span>
 
-react_component('HelloMessage', {:name => 'John'}, {:id => 'hello', :class => 'foo', :tag => :span})
+react_component('HelloMessage', {name: 'John'}, {id: 'hello', class: 'foo', tag: :span})
 # <span class="foo" id="hello" data-...></span>
 ```
 
@@ -118,14 +118,14 @@ gem "therubyrhino", :platforms => :jruby
 
 #### components.js
 
-In order for us to render your React components, we need to be able to find them and load them into the JS VM. By convention, we look for a `assets/components.js` file through the asset pipeline, and load that. For example:
+In order for us to render your React components, we need to be able to find them and load them into the JS VM. By convention, we look for a `assets/javascripts/components.js` file through the asset pipeline, and load that. For example:
 
 ```sass
 // app/assets/javascripts/components.js
 //= require_tree ./components
 ```
 
-This will bring in all files located in the `app/assets/javascripts/components` directory.  You can organize your code however you like, as long as a request for `/assets/components.js` brings in a concatenated file containing all of your React components, and each one has to be available in the global scope (either `window` or `global` can be used). For `.js.jsx` files this is not a problem, but if you are using `.js.jsx.coffee` files then the wrapper function needs to be taken into account:
+This will bring in all files located in the `app/assets/javascripts/components` directory.  You can organize your code however you like, as long as a request for `/assets/javascripts/components.js` brings in a concatenated file containing all of your React components, and each one has to be available in the global scope (either `window` or `global` can be used). For `.js.jsx` files this is not a problem, but if you are using `.js.jsx.coffee` files then the wrapper function needs to be taken into account:
 
 ```coffee
 ###* @jsx React.DOM ###
@@ -139,10 +139,10 @@ window.Component = Component
 
 #### View Helper
 
-To take advantage of server rendering, use the same view helper `react_component`, and pass in `:prerender => true` in the `options` hash.
+To take advantage of server rendering, use the same view helper `react_component`, and pass in `prerender: true` in the `options` hash.
 
 ```erb
-react_component('HelloMessage', {:name => 'John'}, {:prerender => true})
+react_component('HelloMessage', {name: 'John'}, {prerender: true})
 ```
 This will return the fully rendered component markup, and as long as you have included the `react_ujs` script in your page, then the component will also be instantiated and mounted on the client.
 

lib/react/rails/railtie.rb

@@ -19,6 +19,13 @@ class Railtie < ::Rails::Railtie
         app.config.watchable_files.concat Dir["#{app.root}/app/assets/javascripts/**/*.jsx*"]
       end
 
+      # Include the react-rails view helper lazily
+      initializer "react_rails.setup_view_helpers" do
+        ActiveSupport.on_load(:action_view) do
+          include ::React::Rails::ViewHelper
+        end
+      end
+
       # run after all initializers to allow sprockets to pick up react.js and
       # jsxtransformer.js from end-user to override ours if needed
       initializer "react_rails.setup_vendor", :after => "sprockets.environment", group: :all do |app|
@@ -56,13 +63,15 @@ class Railtie < ::Rails::Railtie
       config.after_initialize do |app|
         # Server Rendering
         # Concat component_filenames together for server rendering
-        app.config.react.components_js = app.config.react.component_filenames.map do |filename|
-          app.assets[filename].to_s
-        end.join(";")
+        app.config.react.components_js = lambda {
+          app.config.react.component_filenames.map do |filename|
+            app.assets[filename].to_s
+          end.join(";")
+        }
 
         do_setup = lambda do
           cfg = app.config.react
-          React::Renderer.setup!( cfg.react_js.call, cfg.components_js,
+          React::Renderer.setup!( cfg.react_js, cfg.components_js,
                                 {:size => cfg.size, :timeout => cfg.timeout})
         end
 

lib/react/rails/view_helper.rb

@@ -23,7 +23,3 @@ def react_component(name, args = {}, options = {}, &block)
     end
   end
 end
-
-ActionView::Base.class_eval do
-  include ::React::Rails::ViewHelper
-end

lib/react/renderer.rb

@@ -10,6 +10,7 @@ def self.setup!(react_js, components_js, args={})
       @@react_js = react_js
       @@components_js = components_js
       @@pool.shutdown{} if @@pool
+      reset_combined_js!
       @@pool = ConnectionPool.new(:size => args[:size]||10, :timeout => args[:timeout]||20) { self.new }
     end
 
@@ -19,8 +20,8 @@ def self.render(component, args={})
       end
     end
 
-    def self.combined_js
-      @@combined_js ||= <<-CODE
+    def self.setup_combined_js
+      <<-CODE
         var global = global || this;
 
         var console = global.console || {};
@@ -30,12 +31,20 @@ def self.combined_js
           }
         });
 
-        #{@@react_js};
+        #{@@react_js.call};
         React = global.React;
-        #{@@components_js};
+        #{@@components_js.call};
       CODE
     end
 
+    def self.reset_combined_js!
+      @@combined_js = setup_combined_js
+    end
+
+    def self.combined_js
+      @@combined_js
+    end
+
     def context
       @context ||= ExecJS.compile(self.class.combined_js)
     end

test/helper_files/TodoListWithUpdates.js.jsx

@@ -0,0 +1,22 @@
+/** @jsx React.DOM */
+
+TodoList = React.createClass({
+  getInitialState: function() {
+    return({mounted: "nope"});
+  },
+  componentWillMount: function() {
+    this.setState({mounted: 'yep'});
+  },
+  render: function() {
+    return (
+      <ul>
+        <li>Updated</li>
+        <li id='status'>{this.state.mounted}</li>
+        {this.props.todos.map(function(todo, i) {
+          return (<Todo key={i} todo={todo} />)
+        })}
+      </ul>
+    )
+  }
+})
+

test/react_test.rb

@@ -31,6 +31,8 @@ class ReactTest < ActionDispatch::IntegrationTest
 
     assert_response :success
     assert_equal "'test_confirmation_token_react_content';\n", @response.body
+
+    React::Renderer.reset_combined_js!
   end
 
 end

test/server_rendered_html_test.rb

@@ -0,0 +1,37 @@
+require 'test_helper'
+require 'fileutils'
+
+class ServerRenderedHtmlTest  < ActionDispatch::IntegrationTest
+  # Rails' asset pipeline has trouble picking up changes to files if they happen too fast.
+  # By sleeping for a little bit at certain points, we can make sure that rails notices the
+  # change in the file mtime, and calls our renderer setup functions appropriately
+  def wait_to_ensure_asset_pipeline_detects_changes
+    sleep(1)
+  end
+
+  test 'react server rendering reloads jsx after changes to the jsx files' do
+    file_with_updates = File.expand_path('../helper_files/TodoListWithUpdates.js.jsx', __FILE__)
+    file_without_updates = File.expand_path('../helper_files/TodoListWithoutUpdates.js.jsx', __FILE__)
+    app_file = File.expand_path('../dummy/app/assets/javascripts/components/TodoList.js.jsx', __FILE__)
+
+    FileUtils.cp app_file, file_without_updates
+    FileUtils.touch app_file
+
+    begin
+      get '/server/1'
+      refute_match 'Updated', response.body
+
+      wait_to_ensure_asset_pipeline_detects_changes
+      FileUtils.cp file_with_updates, app_file
+      FileUtils.touch app_file
+
+      get '/server/1'
+      assert_match 'Updated', response.body
+    ensure
+      # if we have a test failure, we want to make sure that we revert the dummy file
+      wait_to_ensure_asset_pipeline_detects_changes
+      FileUtils.mv file_without_updates, app_file
+      FileUtils.touch app_file
+    end
+  end
+end

test/test_helper.rb

@@ -23,3 +23,7 @@
 if ActiveSupport::TestCase.method_defined?(:fixture_path=)
   ActiveSupport::TestCase.fixture_path = File.expand_path("../fixtures", __FILE__)
 end
+
+def wait_for_turbolinks_to_be_available
+  sleep(1)
+end

test/view_helper_test.rb

@@ -62,10 +62,14 @@ class ViewHelperTest < ActionDispatch::IntegrationTest
     page.execute_script('history.back();')
     assert page.has_content?('Hello Alice')
 
+    wait_for_turbolinks_to_be_available()
+
     # Try Turbolinks javascript API.
     page.execute_script('Turbolinks.visit("/pages/2");')
     assert page.has_content?('Hello Alice')
 
+    wait_for_turbolinks_to_be_available()
+
     page.execute_script('Turbolinks.visit("/pages/1");')
     assert page.has_content?('Hello Bob')