closes #155

closes #155

deprecates shallow compare per #156

improved fix for #155

# Conflicts:
#	lib/react/state.rb
#	spec/spec_helper.rb

Author: catmando

.rubocop.yml

@@ -1154,3 +1154,6 @@ Style/WordArray:
   Description: 'Use %w or %W for arrays of words.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#percent-w'
   Enabled: false
+
+Style/CommandLiteral:
+  EnforcedStyle: mixed

lib/rails-helpers/top_level_rails_component.rb

@@ -12,7 +12,7 @@ def self.search_path
     param :controller
     param :render_params
 
-    backtrace :on
+    backtrace :off
 
     def render
       paths_searched = []

lib/react/component.rb

@@ -17,6 +17,7 @@ def self.included(base)
       base.include(Callbacks)
       base.include(Tags)
       base.include(DslInstanceMethods)
+      base.include(ShouldComponentUpdate)
       base.class_eval do
         class_attribute :initial_state
         define_callback :before_mount
@@ -42,34 +43,22 @@ def initialize(native_element)
       @native = native_element
     end
 
-    def render
-      raise "no render defined"
-    end unless method_defined?(:render)
-
-    def update_react_js_state(object, name, value)
-      if object
-        set_state({"***_state_updated_at-***" => Time.now.to_f, "#{object.class.to_s+'.' unless object == self}#{name}" => value})
-      else
-        set_state({name => value})
-      end rescue nil
-    end
-
     def emit(event_name, *args)
-      self.params["_on#{event_name.to_s.event_camelize}"].call(*args)
+      params["_on#{event_name.to_s.event_camelize}"].call(*args)
     end
 
     def component_will_mount
       IsomorphicHelpers.load_context(true) if IsomorphicHelpers.on_opal_client?
       set_state! initial_state if initial_state
       State.initialize_states(self, initial_state)
-      State.set_state_context_to(self) { self.run_callback(:before_mount) }
+      State.set_state_context_to(self) { run_callback(:before_mount) }
     rescue Exception => e
       self.class.process_exception(e, self)
     end
 
     def component_did_mount
       State.set_state_context_to(self) do
-        self.run_callback(:after_mount)
+        run_callback(:after_mount)
         State.update_states_to_observe
       end
     rescue Exception => e
@@ -84,32 +73,6 @@ def component_will_receive_props(next_props)
       self.class.process_exception(e, self)
     end
 
-    def props_changed?(next_props)
-      return true unless props.keys.sort == next_props.keys.sort
-      props.detect { |k, v| `#{next_props[k]} != #{params[k]}`}
-    end
-
-    def should_component_update?(next_props, next_state)
-      State.set_state_context_to(self) do
-        next_props = Hash.new(next_props)
-        if self.respond_to?(:needs_update?)
-          !!self.needs_update?(next_props, Hash.new(next_state))
-        elsif false # switch to true to force updates per standard react
-          true
-        elsif props_changed? next_props
-          true
-        elsif `!next_state != !#{@native}.state`
-          true
-        elsif `!next_state && !#{@native}.state`
-          false
-        elsif `next_state["***_state_updated_at-***"] != #{@native}.state["***_state_updated_at-***"]`
-          true
-        else
-          false
-        end.to_n
-      end
-    end
-
     def component_will_update(next_props, next_state)
       State.set_state_context_to(self) { self.run_callback(:before_update, Hash.new(next_props), Hash.new(next_state)) }
     rescue Exception => e
@@ -136,11 +99,32 @@ def component_will_unmount
 
     attr_reader :waiting_on_resources
 
+    def update_react_js_state(object, name, value)
+      if object
+        name = "#{object.class}.#{name}" unless object == self
+        set_state(
+          '***_state_updated_at-***' => Time.now.to_f,
+          name => value
+        )
+      else
+        set_state name => value
+      end
+    end
+
+    def render
+      raise 'no render defined'
+    end unless method_defined?(:render)
+
     def _render_wrapper
-      State.set_state_context_to(self) do
-        React::RenderingContext.render(nil) {render || ""}.tap { |element| @waiting_on_resources = element.waiting_on_resources if element.respond_to? :waiting_on_resources }
+      State.set_state_context_to(self, true) do
+        element = React::RenderingContext.render(nil) { render || '' }
+        @waiting_on_resources =
+          element.waiting_on_resources if element.respond_to? :waiting_on_resources
+        element
       end
+    # rubocop:disable Lint/RescueException # we want to catch all exceptions regardless
     rescue Exception => e
+      # rubocop:enable Lint/RescueException
       self.class.process_exception(e, self)
     end
 
@@ -151,6 +135,5 @@ def watch(value, &on_change)
     def define_state(*args, &block)
       State.initialize_states(self, self.class.define_state(*args, &block))
     end
-
   end
 end

lib/react/component/class_methods.rb

@@ -6,21 +6,21 @@ module ClassMethods
       def reactrb_component?
         true
       end
-      
+
       def backtrace(*args)
         @dont_catch_exceptions = (args[0] == :none)
         @backtrace_off = @dont_catch_exceptions || (args[0] == :off)
       end
 
-      def process_exception(e, component, reraise = nil)
-        message = ["Exception raised while rendering #{component}"]
-        if e.backtrace && e.backtrace.length > 1 && !@backtrace_off
-          append_backtrace(message, e.backtrace)
-        else
-          message[0] += ": #{e.message}"
+      def process_exception(e, component, reraise = @dont_catch_exceptions)
+        unless @dont_catch_exceptions
+          message = ["Exception raised while rendering #{component}: #{e.message}"]
+          if e.backtrace && e.backtrace.length > 1 && !@backtrace_off
+            append_backtrace(message, e.backtrace)
+          end
+          `console.error(#{message.join("\n")})`
         end
-        `console.error(#{message.join("\n")})`
-        raise e if reraise || @dont_catch_exceptions
+        raise e if reraise
       end
 
       def append_backtrace(message_array, backtrace)

lib/react/component/should_component_update.rb

@@ -0,0 +1,98 @@
+module React
+  module Component
+    #
+    # React assumes all components should update, unless a component explicitly overrides
+    # the shouldComponentUpdate method.  Reactrb does an explicit check doing a shallow
+    # compare of params, and using a timestamp to determine if state has changed.
+
+    # If needed components can provide their own #needs_update? method which will be
+    # passed the next params and state opal hashes.
+
+    # Attached to these hashes is a #changed? method that returns whether the hash contains
+    # changes as calculated by the base mechanism.  This way implementations of #needs_update?
+    # can use the base comparison mechanism as needed.
+
+    # For example
+    # def needs_update?(next_params, next_state)
+    #   # use a special comparison method
+    #   return false if next_state.changed? || next_params.changed?
+    #   # do some other special checks
+    # end
+
+    # Note that beginning in 0.9 we will use standard ruby compare on all params further reducing
+    # the need for needs_update?
+    #
+    module ShouldComponentUpdate
+      def should_component_update?(native_next_props, native_next_state)
+        State.set_state_context_to(self, false) do
+          next_params = Hash.new(native_next_props)
+          # rubocop:disable Style/DoubleNegation # we must return true/false to js land
+          if respond_to?(:needs_update?)
+            !!call_needs_update(next_params, native_next_state)
+          else
+            !!(props_changed?(next_params) || native_state_changed?(native_next_state))
+          end
+          # rubocop:enable Style/DoubleNegation
+        end
+      end
+
+      # create opal hashes for next params and state, and attach
+      # the changed? method to each hash
+
+      def call_needs_update(next_params, native_next_state)
+        component = self
+        next_params.define_singleton_method(:changed?) do
+          component.props_changed?(self)
+        end
+        next_state = Hash.new(native_next_state)
+        next_state.define_singleton_method(:changed?) do
+          component.native_state_changed?(native_next_state)
+        end
+        needs_update?(next_params, next_state)
+      end
+
+      # Whenever state changes, reactrb updates a timestamp on the state object.
+      # We can rapidly check for state changes comparing the incoming state time_stamp
+      # with the current time stamp.
+
+      # Different versions of react treat empty state differently, so we first
+      # convert anything that looks like an empty state to "false" for consistency.
+
+      # Then we test if one state is empty and the other is not, then we return false.
+      # Then we test if both states are empty we return true.
+      # If either state does not have a time stamp then we have to assume a change.
+      # Otherwise we check time stamps
+
+      # rubocop:disable Metrics/MethodLength # for effeciency we want this to be one method
+      def native_state_changed?(next_state)
+        %x{
+          var current_state = #{@native}.state
+          var normalized_next_state =
+            !#{next_state} || Object.keys(#{next_state}).length === 0 || #{nil} == #{next_state} ?
+            false : #{next_state}
+          var normalized_current_state =
+            !current_state || Object.keys(current_state).length === 0 || #{nil} == current_state ?
+            false : current_state
+          if (!normalized_current_state != !normalized_next_state) return(true)
+          if (!normalized_current_state && !normalized_next_state) return(false)
+          if (!normalized_current_state['***_state_updated_at-***'] ||
+              !normalized_next_state['***_state_updated_at-***']) return(true)
+          return (normalized_current_state['***_state_updated_at-***'] !=
+                  normalized_next_state['***_state_updated_at-***'])
+        }
+      end
+      # rubocop:enable Metrics/MethodLength
+
+      # Do a shallow compare on the two hashes. Starting in 0.9 we will do a deep compare.
+
+      def props_changed?(next_params)
+        Component.deprecation_warning(
+          "Using shallow incoming params comparison.\n"\
+          'Do a require "reactrb/deep-compare, to get 0.9 behavior'
+        )
+        (props.keys.sort != next_params.keys.sort) ||
+          next_params.detect { |k, v| `#{v} != #{@native}.props[#{k}]` }
+      end
+    end
+  end
+end

lib/react/state.rb

@@ -33,26 +33,34 @@ def method_missing(method, *args)
   end
 
   class State
+
+    @rendering_level = 0
+
     class << self
       attr_reader :current_observer
 
-      def initialize_states(object, initial_values)
-        # initialize objects' name/value pairs
+      def initialize_states(object, initial_values) # initialize objects' name/value pairs
         states[object].merge!(initial_values || {})
       end
 
       def get_state(object, name, current_observer = @current_observer)
-        # get current value of name for object, remember that the current object
-        # depends on this state, current observer can be overriden with last
-        # param
-        new_observers[current_observer][object] << name if current_observer &&
-          !new_observers[current_observer][object].include?(name)
+        # get current value of name for object, remember that the current object depends on this state,
+        # current observer can be overriden with last param
+        new_observers[current_observer][object] << name if current_observer && !new_observers[current_observer][object].include?(name)
         states[object][name]
       end
 
-      def set_state2(object, name, value)
-        # set object's name state to value, tell all observers it has changed.
-        # Observers must implement update_react_js_state
+      def set_state(object, name, value, wait_till_thread_completes = nil)
+        states[object][name] = value
+        if wait_till_thread_completes
+          notify_observers_after_thread_completes(object, name, value)
+        elsif @rendering_level == 0
+          notify_observers(object, name, value)
+        end
+        value
+      end
+
+      def notify_observers(object, name, value)
         object_needs_notification = object.respond_to? :update_react_js_state
         observers_by_name[object][name].dup.each do |observer|
           observer.update_react_js_state(object, name, value)
@@ -61,23 +69,14 @@ def set_state2(object, name, value)
         object.update_react_js_state(nil, name, value) if object_needs_notification
       end
 
-      def set_state(object, name, value, delay=nil)
-        states[object][name] = value
-        if delay
-          @delayed_updates ||= []
-          @delayed_updates << [object, name, value]
-          @delayed_updater ||= after(0.001) do
-            delayed_updates = @delayed_updates
-            @delayed_updates = []
-            @delayed_updater = nil
-            delayed_updates.each do |object, name, value|
-              set_state2(object, name, value)
-            end
-          end
-        else
-          set_state2(object, name, value)
+      def notify_observers_after_thread_completes(object, name, value)
+        (@delayed_updates ||= []) << [object, name, value]
+        @delayed_updater ||= after(0) do
+          delayed_updates = @delayed_updates
+          @delayed_updates = []
+          @delayed_updater = nil
+          delayed_updates.each { |args| notify_observers(*args) }
         end
-        value
       end
 
       def will_be_observing?(object, name, current_observer)
@@ -88,9 +87,7 @@ def is_observing?(object, name, current_observer)
         current_observer && observers_by_name[object][name].include?(current_observer)
       end
 
-      # should be called after the last after_render callback, currently called
-      # after components render method
-      def update_states_to_observe(current_observer = @current_observer)
+      def update_states_to_observe(current_observer = @current_observer)  # should be called after the last after_render callback, currently called after components render method
         raise "update_states_to_observer called outside of watch block" unless current_observer
         current_observers[current_observer].each do |object, names|
           names.each do |name|
@@ -116,23 +113,21 @@ def remove # call after component is unmounted
         current_observers.delete(@current_observer)
       end
 
-      # wrap all execution that may set or get states in a block so we know
-      # which observer is executing
-      def set_state_context_to(observer)
+      def set_state_context_to(observer, rendering = nil) # wrap all execution that may set or get states in a block so we know which observer is executing
         if `typeof window.reactive_ruby_timing !== 'undefined'`
           @nesting_level = (@nesting_level || 0) + 1
           start_time = Time.now.to_f
           observer_name = (observer.class.respond_to?(:name) ? observer.class.name : observer.to_s) rescue "object:#{observer.object_id}"
         end
         saved_current_observer = @current_observer
         @current_observer = observer
+        @rendering_level += 1 if rendering
         return_value = yield
         return_value
       ensure
         @current_observer = saved_current_observer
-        if `typeof window.reactive_ruby_timing !== 'undefined'`
-          @nesting_level = [0, @nesting_level - 1].max
-        end
+        @rendering_level -= 1 if rendering
+        @nesting_level = [0, @nesting_level - 1].max if `typeof window.reactive_ruby_timing !== 'undefined'`
         return_value
       end
 
@@ -142,7 +137,7 @@ def states
 
       [:new_observers, :current_observers, :observers_by_name].each do |method_name|
         define_method(method_name) do
-          instance_variable_get("@#{method_name}") or
+          instance_variable_get("@#{method_name}") ||
           instance_variable_set("@#{method_name}", Hash.new { |h, k| h[k] = Hash.new { |h, k| h[k] = [] } })
         end
       end

lib/reactrb.rb

@@ -20,6 +20,7 @@
   require 'react/observable'
   require 'react/component'
   require 'react/component/dsl_instance_methods'
+  require 'react/component/should_component_update'
   require 'react/component/tags'
   require 'react/component/base'
   require 'react/element'