closes #152

Author: catmando

lib/react/component/class_methods.rb

@@ -2,6 +2,11 @@ module React
   module Component
     # class level methods (macros) for components
     module ClassMethods
+
+      def reactrb_component?
+        true
+      end
+      
       def backtrace(*args)
         @dont_catch_exceptions = (args[0] == :none)
         @backtrace_off = @dont_catch_exceptions || (args[0] == :off)

lib/react/rendering_context.rb

@@ -13,24 +13,12 @@ def self.build_only(name, *args, &block)
 
     def self.render(name, *args, &block)
       remove_nodes_from_args(args)
-      @buffer = [] unless @buffer
+      @buffer ||= [] unless @buffer
       if block
         element = build do
           saved_waiting_on_resources = waiting_on_resources
           self.waiting_on_resources = nil
-          result = block.call
-          # Todo figure out how children rendering should happen, probably should have special method that pushes children into the buffer
-          # i.e. render_child/render_children that takes Element/Array[Element] and does the push into the buffer
-          if !name && (  # !name means called from outer render so we check that it has rendered correctly
-              (@buffer.count > 1) || # should only render one element
-              (@buffer.count == 1 && @buffer.last != result) || # it should return that element
-              (@buffer.count == 0 && !(result.is_a?(String) || (result.respond_to?(:acts_as_string?) && result.acts_as_string?) || result.is_a?(Element))) #for convience we will also convert the return value to a span if its a string
-            )
-            raise "a components render method must generate and return exactly 1 element or a string"
-          end
-
-          @buffer << result.to_s if result.is_a? String || (result.respond_to?(:acts_as_string?) && result.acts_as_string?) # For convience we push the last return value on if its a string
-          @buffer << result if result.is_a?(Element) && @buffer.count == 0
+          run_child_block(name.nil?, &block)
           if name
             buffer = @buffer.dup
             React.create_element(name, *args) { buffer }.tap do |element|
@@ -44,7 +32,6 @@ def self.render(name, *args, &block)
         end
       elsif name.is_a? React::Element
         element = name
-        # I BELIEVE WAITING ON RESOURCES SHOULD ALREADY BE SET
       else
         element = React.create_element(name, *args)
         element.waiting_on_resources = waiting_on_resources
@@ -78,6 +65,51 @@ def self.remove_nodes_from_args(args)
         value.as_node if value.is_a?(Element) rescue nil
       end if args[0] && args[0].is_a?(Hash)
     end
+
+    # run_child_block gathers the element(s) generated by a child block.
+    # for example when rendering this div: div { "hello".span; "goodby".span }
+    # two child Elements will be generated.
+    #
+    # the final value of the block should either be
+    #   1 an object that responds to :acts_as_string?
+    #   2 a string,
+    #   3 an element that is NOT yet pushed on the rendering buffer
+    #   4 or the last element pushed on the buffer
+    #
+    # in case 1 we change the object to a string, and then it becomes case 2
+    # in case 2 we automatically push the string onto the buffer
+    # in case 3 we also push the Element onto the buffer IF the buffer is empty
+    # case 4 requires no special processing
+    #
+    # Once we have taken care of these special cases we do a check IF we are in an
+    # outer rendering scope.  In this case react only allows us to generate 1 Element
+    # so we insure that is the case, and also check to make sure that element in the buffer
+    # is the element returned
+
+    def self.run_child_block(is_outer_scope)
+      result = yield
+      result = result.to_s if result.try :acts_as_string?
+      @buffer << result if result.is_a?(String) || (result.is_a?(Element) && @buffer.empty?)
+      raise_render_error(result) if is_outer_scope && @buffer != [result]
+    end
+
+    # heurestically raise a meaningful error based on the situation
+
+    def self.raise_render_error(result)
+      improper_render 'A different element was returned than was generated within the DSL.',
+                      'Possibly improper use of Element#delete.' if @buffer.count == 1
+      improper_render "Instead #{@buffer.count} elements were generated.",
+                      'Do you want to wrap your elements in a div?' if @buffer.count > 1
+      improper_render "Instead the component #{result} was returned.",
+                      "Did you mean #{result}()?" if result.try :reactrb_component?
+      improper_render "Instead the #{result.class} #{result} was returned.",
+                      'You may need to convert this to a string.'
+    end
+
+    def self.improper_render(message, solution)
+      raise "a component's render method must generate and return exactly 1 element or a string.\n"\
+            "    #{message}  #{solution}"
+    end
   end
 
   class ::Object

spec/react/component_spec.rb

@@ -495,6 +495,58 @@ def render; "hello" end
     end
   end
 
+  describe 'Render Error Handling' do
+    before(:each) do
+      %x{
+        window.test_log = [];
+        window.org_warn_console = window.console.warn;
+        window.org_error_console = window.console.error
+        window.console.warn = window.console.error = function(str){window.test_log.push(str)}
+      }
+    end
+    it "will generate a message if render returns something other than an Element or a String" do
+      foo = Class.new(React::Component::Base)
+      foo.class_eval do
+        def render; Hash.new; end
+      end
+
+      renderToDocument(foo)
+      `window.console.warn = window.org_warn_console; window.console.error = window.org_error_console;`
+      expect(`test_log`.first).to match /Instead the Hash \{\} was returned/
+    end
+    it "will generate a message if render returns a Component class" do
+      stub_const 'Foo', Class.new(React::Component::Base)
+      foo = Class.new(React::Component::Base)
+      foo.class_eval do
+        def render; Foo; end
+      end
+
+      renderToDocument(foo)
+      `window.console.warn = window.org_warn_console; window.console.error = window.org_error_console;`
+      expect(`test_log`.first).to match /Did you mean Foo()/
+    end
+    it "will generate a message if more than 1 element is generated" do
+      foo = Class.new(React::Component::Base)
+      foo.class_eval do
+        def render; "hello".span; "goodby".span; end
+      end
+
+      renderToDocument(foo)
+      `window.console.warn = window.org_warn_console; window.console.error = window.org_error_console;`
+      expect(`test_log`.first).to match /Instead 2 elements were generated/
+    end
+    it "will generate a message if the element generated is not the element returned" do
+      foo = Class.new(React::Component::Base)
+      foo.class_eval do
+        def render; "hello".span; "goodby".span.delete; end
+      end
+
+      renderToDocument(foo)
+      `window.console.warn = window.org_warn_console; window.console.error = window.org_error_console;`
+      expect(`test_log`.first).to match /A different element was returned than was generated within the DSL/
+    end
+  end
+
   describe 'Event handling' do
     before do
       stub_const 'Foo', Class.new