hyperstack-org / hyperstack

Hyperstack ALPHA https://hyperstack.org
https://docs.hyperstack.org
MIT License
540 stars 41 forks source link

params which contain procs (callbacks) cause re-rerenders #329

Open catmando opened 3 years ago

catmando commented 3 years ago

class Child
  include Hyperstack::Component
​
  param :do_something
​
  render do
    puts "child: @DoSomething.object_id"
  end
end
​
class Parent
  include Hyperstack::Component
​
  before_mount do
    @do_something = -> { puts "we did it" }
  end
​
  render do
    puts "parent: @do_something.object_id"
    Child(do_something: @do_something)
  end
end

the two object_id's won't match, causing the child to be rerendered everytime the parent is rendered.

Clearly something is wrapping proc params, but not clear where this is happening.

lionelchauvin commented 3 years ago

on my projet I fixed this problem by redefining props_changed?

  def props_changed?(next_props)
    return true if `Object.keys(#{@__hyperstack_component_native}.props).length` != next_props.length
    props = Hash.new(`#{@__hyperstack_component_native}.props`)
    next_props.each do |k, v|
      next if v.is_a?(Proc) && props[k].is_a?(Proc)
      return true if props[k] != v
    end
    return false
  end

I am not really satisfied with Hash.new A native solution would be better but I don't know how to check if a value is a Proc in javascript

princejoseph commented 3 years ago

@catmando Usually we do fires for this kind of callback props instead of params, right?

catmando commented 3 years ago

@princejoseph yes but i think fires is just syntactic sugar ontop of the base mechanism, so the performance issue will exists regardless.

catmando commented 3 years ago

More info on this from @adamcreekroad:

The problem is that some where along the line a proc object is being wrapped with an extra JS function as its being passed around this makes the proc look like a new proc object every time.

princejoseph commented 3 years ago

@catmando ah, makes total sense. I really liked that it has a seperate fires for the component. Makes the component behaviour obvious in a glance.

catmando commented 3 years ago

Note: Spec is already written, and is being skipped in component_spec.rb