ruby-hyperloop / hyper-react

The project has moved to Hyperstack!!
https://hyperstack.org/
MIT License
285 stars 14 forks source link

automate .as_node.to_n should be applied when elements are passed to native components. #122

Open sollycatprint opened 8 years ago

sollycatprint commented 8 years ago

From @catmando on January 21, 2016 23:52

currently if you do this:

  # Rb is native ReactBootstrap 
  class TooltipExample < React::Component::Base
    def render
      tip = Rb.Tooltip(id: 'foo'){'a tooltip'}.
      div {
        h2 { 'Tooltip Example'}
        Rb.ButtonToolbar {
          MyComponent(placement: :top, overlay: tip) {
            Rb.Button { 'has tooltip' }
          }
          Rb.Button { 'no tooltip' }
        }
      }
    end
  end

It works fine: I.e the tip Element is automatically removed from the rendering buffer when it is passed to MyComponent.

But if you change MyComponent to a native component, you have to remember to remove the node, and convert it to a native JS object.

  class TooltipExample < React::Component::Base
    def render
      tip = Rb.Tooltip(id: 'foo'){'a tooltip'}.as_node.to_n
      div {
        h2 { 'Tooltip Example'}
        Rb.ButtonToolbar {
          Rb.OverlayTrigger(placement: :top, overlay: tip) {
            Rb.Button { 'has tooltip' }
          }
          Rb.Button { 'no tooltip' }
        }
      }
    end
  end

This is because the React::RenderingContext.remove_nodes_from_args method is called at the beginning of React::RenderingContext.render. Instead we should do this processing when args are PASSED, and during the check do the .to_n processing if the component is native.

Copied from original issue: zetachang/react.rb#122

catmando commented 8 years ago

@zetachang this might be fixed... not sure...

zetachang commented 7 years ago

The native component referred is something imported by React::NativeLibrary?

catmando commented 7 years ago

@zetachang it could be imported by React::NativeLibrary (or the auto import mechanism) that is the normal way. However you might for some weird reason manually import it. The point is that right now all HyperReact components begin rendering by checking and removing any nodes passed in from the parents buffer.

This means that if you pass a node to a native component (regardless of how you get access to it) that node will NOT be removed from the buffer.

So the solution seems to be either: (1) remove nodes when params are passed, or (2) at least remove nodes when params are passed to a native component.

Like I said it could be that this is fixed. I would just put a test case in, and see what happens!

catmando commented 7 years ago

another approach maybe is to make Element#to_n convert to native AND remove from the buffer.

that way this problem becomes simply doing a to_n to all values passed a native component which would be nice!