Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Property inheritance (Scala→Scala) #2

Closed
dispalt opened this issue Jul 8, 2014 · 38 comments
Closed

Property inheritance (Scala→Scala) #2

dispalt opened this issue Jul 8, 2014 · 38 comments

Comments

@dispalt
Copy link
Contributor

dispalt commented Jul 8, 2014

I was curious how you'd approach the following problem: I've been using React-bootstrap for quite some time and was looking to use it with your library. I am still new to fully understanding how your library works, but my thought was to use it similarly to how you use the animation addon. So that would convert something like this to something abbreviated like

case class Button(active: js.UndefOr[Boolean] = js.undefined,
    onclick: js.UndefOr[() => Unit] = js.undefined) {
  def toJs: js.Object = {
    val p = lit("active" -> active)
    onclick.foreach(f => p.updateDynamic("onClick")(f))
    p
  }

  def apply(children: js.Any*): ReactComponentU_ = {
    val f = ReactBootstrap.Button
    f(toJs, js.Array(children.toSeq: _*)).asInstanceOf[ReactComponentU_]
  }
}

Again it's abbreviated, assume ReactBootstrap is a dynamic. Also on the apply function I wanted to have bare children, pure text, so I widened whats expected but I didn't spend enough time to try to figure out how all the implicits would help me with narrowing that scope.

Thanks for the help.
-Dan

@aappddeevv
Copy link

We need to be able to gain access to the automagic children property on the "properties" object found in js react. We need that access in the render method.

React-bootstrap is a good example of the using the children feature. Here's a specific piece of code that highlights its usage and why its important: https://github.com/react-bootstrap/react-bootstrap/blob/master/src/Panel.jsx.

Here's some code that highlights the issue:

val Search = ReactComponentB[String] { ... }

 val Panel = ReactComponentB[PanelProps]("Panel")
    .initialState(PanelState(true))
    .render { (props, state) =>
      // How do we access the children and add it to
      // the scalatags expression below?
      div()()
    }
    .create

in another component we would create something like:

.render {... =>
...
 Panel(PanelProps("Search"),
          Search("")) // Search is added as a "child" of the react component, not in the scalatags hier.
...
}

Probably the best way to solve this is to figure out how to get the "this" pointer into the render method then you could access any of the properties in a raw fashion, like the children property on the this.props object. And this would allow any other hacks needed. Then a nice mechanism could be put in place. Perhaps a B5?

@dispalt
Copy link
Contributor Author

dispalt commented Jul 9, 2014

I guess there are two separate issues here:

  1. The current component structure doesn't allow the special children property to be exposed easily.
  2. The Panel class is already "created" in that library, so I just need to instantiate it, and what's the best way to do that? I don't want to override the state or the render method, just need to pass in the appropriate props...

@dispalt
Copy link
Contributor Author

dispalt commented Jul 9, 2014

B5 might be hard to do that with because a lot of simple components, ones without state, use the children modifier.

@aappddeevv
Copy link

For the particular react-bootstrap its already there but we need that capability directly.

I hacked the builder class very quickly to allow me to get access to the react object directly in the render method as a 4th argument assuming the first few were already defined. I'm trying to figure out how to slice an already instantiated react component into the scalatags so its lightly "rendered" correctly. I'm having trouble figuring out how to have a vdom builder just return the react component since it already exists.

My guess is that there needs to be a general way to get the "component" object injected as a js.Any or js.Dynamic into some of the methods in case you need it. I'm sure that there other things in react that need atttention.

@dispalt
Copy link
Contributor Author

dispalt commented Jul 9, 2014

I did something like this to use a mixin which forces me to add a renderOverlay function

private val _bspec = ReactComponentB[Unit]("ApplicationModalTrigger")
    .initialState(AMTState(open = false))
    .backend(new AMTBackend(_))
    .render { (_, S, B) =>
      Button(active = false, bsStyle = "primary", onclick = B.toggle)("+")
    }.buildSpec

  { // TODO: Goofy
    val renderOverlay: ComponentScopeU[Unit, AMTState, AMTBackend] => VDom = PSB => {
      dom.console.log(PSB.state.open)
      if (PSB.state.open)
        ApplicationCreateModal(PSB.backend.hide)
      else
        span()
    }

    val uspec = _bspec.asInstanceOf[js.Dynamic]
    uspec.updateDynamic("mixins")(js.Array(ReactBootstrap.OverlayMixin))
    uspec.updateDynamic("renderOverlay")(renderOverlay: ThisFunction)
  }

val ApplicationModalTrigger = new WComponentConstructor(React.createClass(_bspec))

edited: Left out some code.

@aappddeevv
Copy link

Oh wow, that's alot. I changed the render to give me a "this" parameter where this is the react component. From there, you can do everything you need inside of render. I've confirmed that I have the children in the this.props.children correctly and such. I just need to slice it. I'm thinking that "this" should be the first arg in the methods or that the methods should take a context that has the individual objects in it, including this. That way each method takes a single context object and a significant amount of manipulation can occur on it in the background. Right now, with just the individual parameters being unwrapped and passed in, you are kind of stuck. You still get type safety but you gain flexibility to adapt with less impact on code. Essentially, the "scope" object (which is essentially a context) is passed in to the method. It turns out the scope object is actually the react component as near as I can tell so maybe that's all that's really needed.

@aappddeevv
Copy link

I just got it to work, this was the cast that I needed.

ths.props.children.asInstanceOf[ReactComponentU[_,_,_]]

ths is passed in as the last argument to render and represents the actual react component.

Maybe react-bootstrap can be aliased using js.Object and @JsName to get access to the components. I'm not sure what the signature would be though to make their use seamless in scalajs-react though.

@aappddeevv
Copy link

It looks like you may be able to avoid the ths argument by rigging your Backend to contain the react component then doing casting in render. It's hacky but it may work.

That keeps the current API intact until a better type-safe way is found. I'll give this a try. There may be no good answer to the this.props.children given its an opaque type. I did define some React.Children methods though to work with the opaque type and make that easier.

@aappddeevv
Copy link

It looks like the approach works. Lots of casts...no code changes needed though....but an approach to children management and rendering needs to be worked out. The children can be adhocly set almost anywhere.

I am rebuilding the react-bootstrap Panel as a test for scalajs-react. Notice the children function below. As soon as I get the Panel worked out I'll post it as an example.

  class PanelBackend(scope: ComponentScopeB[PanelProps, PanelState]) {
    def children: js.Any = scope._props.asInstanceOf[js.Dynamic].children
    var isChanging: Boolean = false
    def handleSelect(e: SyntheticEvent[HTMLInputElement]): Unit = {
      g.console.log("clicked on panel header")
      for {
        func <- scope.props.onClick
        key <- scope.props.key
      } yield {
        isChanging = true
        func(key)
        isChanging = false
      }
      e.preventDefault()
      scope.setState(scope.state.copy(expanded = !scope.state.expanded))
    }
  }

@dispalt
Copy link
Contributor Author

dispalt commented Jul 10, 2014

To make a sorta sane way to add properties to a ReactComponentB I wrap the builder to ReactComponentB with a class that looks something like this:

class EnrichedReactComponent[Props, State, Backend](cb: ReactComponentB[Props]#B2[State]#B3[Backend]#B4) {
  var specAdditions = Map.empty[String, js.Any]

  def property(name: String, thing: js.Any) = {
    specAdditions += name -> thing
    this
  }

  def create = {
    val spec = cb.buildSpec
    val dspec = spec.asInstanceOf[js.Dynamic]
    specAdditions.foreach {
      case (k, v) => dspec.updateDynamic(k)(v)
    }
    new WComponentConstructor(React.createClass(spec))
  }
}

object EnrichedReactComponent {
  def apply[Props, State, Backend](rc: ReactComponentB[Props]#B2[State]#B3[Backend]#B4) = new EnrichedReactComponent[Props, State, Backend](rc)
}

@japgolly
Copy link
Owner

Hey guys, I've got a bit of time this afternoon so I'll have a look at this now. Although I plan to use React and Scala.js in my own project I'm not at that stage yet so my experience is limited at this point. Skimming the comments here I don't really get it yet, but I'll slow down. Do either of you have a simple JS example with no deps (so without bootstrap for now)?

@japgolly
Copy link
Owner

Getting up to speed. Looking at http://facebook.github.io/react/tips/children-props-type.html, it seems that this.props.children is just the arguments that are passed into the component's constructor. That's it? I was thinking it was supposed to be a collection of components created in the render phase or something.

japgolly added a commit that referenced this issue Jul 10, 2014
(Makes life easier when React exposes something I don't know about, like
in #2)
@dispalt
Copy link
Contributor Author

dispalt commented Jul 10, 2014

Here's a simple example of using props.children. If I wanted to represent a well component in JSX I would do something like this:

/** @jsx React.DOM */
<Well>
  <Well>Look I am a well.</Well>
  <MoreWell></MoreWell>
</Well>

Which would compile to

/** @jsx React.DOM */
Well(null,
  Well(null, "Look I am a well."),
  MoreWell(null)
)

And basically any properties beyond the first property get mapped to this.props.children. So then in the render component of the well, you could display the children properties any way you'd like. It's up to the component creator. The children props can either be an array of more > 1 or the (== 1) direct child.

@japgolly
Copy link
Owner

Cool guys, I got it now. I believe I know what to do and how to do it nicely.

@japgolly
Copy link
Owner

Thanks for the example, Dan. That would've slightly confused me cos it doesn't use this.props.children directly but I get it now. The bootstrap examples helped. This is the kind of scenario we're talking about, right?

var T2 = React.createClass({displayName: 'T2',
  componentDidMount: function() {
    console.log("T2", this, this.props.children);
  },  
  render: function() {
    return React.DOM.div(null ,this.props.children);
  }
});

React.renderComponent(
  T2(null, React.DOM.h1(null,"h1"),React.DOM.h2(null,"h2"),React.DOM.h3(null,"h3")),
  document.getElementById("t2")
);

@japgolly
Copy link
Owner

Need to switch tasks. I'll get this updated later tonight so hopefully some good news for you tomorrow morning. Cheers.

@dispalt
Copy link
Contributor Author

dispalt commented Jul 10, 2014

Yeah exactly. Good to hear, thanks again for the great library.

japgolly added a commit that referenced this issue Jul 10, 2014
@japgolly
Copy link
Owner

Alright guys, have a try now.

  1. There are overloaded render methods that optionally expose the children.
  2. The children can be used directly inside the Scalatags stuff.
  3. Added React.Children but recommend that you use... (↓)
  4. Added nice only and forEach extensions to the props children.

See the tests for examples.

(And thanks for the kind words, Dan; glad to hear it's useful!)

@aappddeevv
Copy link

I'll give it a try. Thank you for looking into it.

As I worked through some examples, I found its not just about exposing the children, but also about transferring the properties "down" into them. The transferPropertiesTo seems to be critical or encapsulation. The issue then becomes, how you do transfer properties to them in a type safe way when the properties object is strongly typed and you are passing them to the children. If you know the react component and the exact type of the "child" then you can use code to convert your properties into the child's properties. But for the code snippet below, react uses js ability quite nicely to blindly pass those properties.

Here's a snippet that on react-bootstrap that illustrates this point. As I went through some scenarios that I'll also hit, I ran into this problem, specifically around setting attributes and styles derived from properties:

var Jumbotron = React.createClass({

  render: function () {
    return this.transferPropsTo(
      <div className='jumbotron'>
        {this.props.children}
      </div>
    );
  }
});

The other issue is that you can add children on the fly as well simply by copying the react component as a js object then adding the "children" property with some more components. I've seen this trick in some react components but I'm not convinced that there's not a more sane approach around it until we play with it more.

@aappddeevv
Copy link

I just pulled down the changes and had a look.

One thought for you is instead of complicating the render api for future generations, perhaps just add the children to and allow access through any object that uses this trait. That way the children are always available in other areas of the API.

trait ComponentScope_P[Props] extends Object {
  @JSName("props") def _props: WrapObj[Props] = ???
  @JSName("props.children") def children: XXX
}

P.S. I think the dotted name props.children will work here.

Thanks for being so responsive around these issues.

I like Dan's example of enriching those properties, but then we lose the type safeness of the properties being a scala object. But maybe that is the trade-off we have to make.

@japgolly
Copy link
Owner

Hey Devon. The overloaded render methods are just for convenience but I did also expose the entire this object as you suggested just in case. You can now receive a ScopeU (as I call it) and then instead of .props.children, you can use .propsChildren.

Regarding the transferPropsTo thing, I'll have a look at some examples. If a child accepts something from a parent then static typing shouldn't prevent it, just clarify the dependency. We should be able to do it in a nice way; let's see how it goes.

@aappddeevv
Copy link

That's probably my fault when I read the patch. I thought I read it correctly, but I clicked on it again and it was much larger so I did not read all of it :-)

I think static typing won't prevent it, but how you expose properties to the children while keeping it usable is the key question. For js authored components, the scala-authored properties need to be convertible to a js object with a simple key-value structure. For scala authored components, its a scala typed object that would get passed down into a scala-created object but then the wrappers may not match up for the user-declared property types. More on this topic here: http://facebook.github.io/react/docs/reusable-components.html. That example about slightly enhancing the link and such were spot on for me.

var CheckLink = React.createClass({
  render: function() {
    // transferPropsTo() will take any props passed to CheckLink
    // and copy them to <a>
    return this.transferPropsTo(<a>{' '}{this.props.children}</a>);
  }
});

@aappddeevv
Copy link

I created some notes on this topic so its a bit long...not sure its all true yet.

I've been looking at the properties handling and in general, its pretty messy in react because it makes a huge assumption that the properties object is just a js object and has consistency in all react components. In scala, they are typed.

I think some of the following, if not all, are true:

  • Initial properties can be specified in the create call and those are "merged" into any properties provided by the caller. An initial property object may be cached by the framework. We don't really need this but it could be nice to have a default properties object, with default properties, accessible as a member on the react component in scala. That way no companion or naming convention is needed to find default properties.
  • Properties are also copied (deep or shallow?) from an owner component down to the child component. The owner's properties sent to the child via transferPropertiesTo. Some merging can occur with the child's properties as well as specific owner properties. This is equivalent to hierarchical scope or instance variables that span the logical tree capabilities in other UI frameworks. This is a critical capability to allow "settings" to cascade down the component tree. When transferPropertiesTo, the transfer strategy (the design pattern in FB react) determines just how much to copy in the transfer. Some filtering actually occurs in the framework automatically. The transfer strategy is maintained in a React framework singleton TransferStrategies object.
  • Properties are added to, as we have seen, by the framework, to handle children and potentially other things that are still unknown. However, it is known that the raw js properties object holds
    multiple properties used by the framework including refs and style and some of these are copied down.
  • There are a few owner => child scenarios to consider to make sure the properties are handled correctly when passed down through transferPropertiesTo:
    • js => scala: raw js object has properties that must be serialized into a scala properties. This scenario also covers the interop scenario that Dan is looking into.
    • scala => js: js requires properties to be directly on raw js properties object, scala could deserialize them from scala to js
    • scala => scala: not clear this is handled yet, since types will be unknown from owner to child, properties need to be deserialized to the js properties object, then serialized back into the child's scala property type
    • js => js: already handled with no scala usage issues

The transferPropertiesTo is to save typing of you having to copy them, but in scala, you can't do that really because properties have structure and are wrapped (scala object attached to the actual properties js object via the "v" member). So to replicate this, you have to manually take one component's properties and write the code to translate them into the child's specific type. However, as we have seen, sometimes you may not know the specific type because they are hidden in the props.children opaque structure. This implies that some type of serialization or deserialization is needed at the API level, say, to serialize from a scala typed properties object in the owner (whatever the programmer has defined) => javascript => scala typed properties object in the child. You can't always know what the owner component will be and hence you have to think serialization versus anything else.

I need to look into your API but there is a good chance that properties passed in via the owner under certain scenarios will fail to represent the scala typed property object in the child. This is true for componentWillReceiveProps and for the property scope trait. Issues may occur when transferPropsTo is used between js and scala if the transferPropsTo is called (as a hack at the scala level since its not in your API).

I may be all wet in my thinking on this, but I don't know how else to get an owner's component properties copied down to a child's component's properties without some sort of intermediate format. You would need to add a new deserializaiton API function to the builder with signature (Props, js.Dynamic) => Props where Props is the child's scala property object type. A type class may also work here. The owner, whenever transferPropsTo is called, would need to explicitly augment the actual js properties object. The owner could do this anyway it wants to but the easiest way would be to take each scala properties object property, and add its key+value to the actual js properties object. Then the deserialization function above would take the original scala properties object and the actual js properties object and deserialize it to a new scala properties object that becomes the current properties value.

Both serialization and deserialization need to use the actual js react properties object as the interim point and most importantly ensure that there is proper cleanup on it by the parent component once the transfer is complete. For example, if the js properties object was augmented by adding keys representing the properties in the scala properties object, the parent would need to remove those keys after the transfer. You have to use the actual js react properties object because there are other values on it that are copied by the framework and its how it operates underneath--its extremely explicit. It's alot like Dan's first comment where he is targeting the raw js properties object when he adds his constructor arguments. That's actually a serialization step.

The programmer would have to ensure that transferPropertiesTo is only called on a react component and not a scalatags object in their code. That's because the scalatags objects are transformed at run-time to the vdom objects and hence scalatag objects have no concept of the properties object. In JSX, you can wrap the entire JSX expression and my guess is that the JSX compiler provides some clever wrapping.

I believe this is alot like how the backbone react component mixin operates. Only in that case, they can just "set" the react js property value directly because the backbone model is also js. So its a simpler protocol. https://github.com/magalhas/backbone-react-component/blob/master/lib/component.js

@japgolly
Copy link
Owner

FYI I just published a new release (0.2.0) with the propsChildren changes in it. Will take 2hrs to sync but then you'll be able to grab it direct off Maven central and can remove the bintray crap from your builds.

@aappddeevv
Copy link

I'm still working on making transferPropsTo work. I'm still testing the scenarios to make sure the properties are transferred. There may be some subtle property object merging that go on. The general process I outlined above appears to work but I am having trouble with a few issues. This should allow us to transfer properties through the DOM trees.

@japgolly
Copy link
Owner

Hey Devon, interesting. Good to hear you're making progress. I'd like to see what you've come up with. I haven't given prop transfer any attention yet (maybe this weekend).

@aappddeevv
Copy link

The area where I am having problems is the deserialization. It looks like the react js object does not have the additional methods I added when its state is UNMOUNTED. However, once MOUNTED, the deserializeProps and mergeProps are there. So when I try to find the deserializeProps method on the target object during transferPropsTo, it can't find it. It finds the mergeProps fine though.

Here's the hacked transferPropertiesTo method

def transferPropsTo2(src: js.Any, target: js.Any): ReactComponentU[_, _, _] = {
    g.console.log("general transferPropsTo called")
    val s = src.asInstanceOf[js.Dynamic]
    val t = target.asInstanceOf[js.Dynamic]
    g.console.log("source object:  " + pobj(s))
    g.console.log("source properties object: " + pobj(s.props))
    g.console.log("target object: " + pobj(t))
    g.console.log("target properties object: " + pobj(t.props))
    var carryMeThrough: Any = js.undefined
    if (s.mergeProps != js.undefined) {
      g.console.log("source has mergeProps")
      if (s.props.v != js.undefined) {
        g.console.log("source has a props object, calling mergeProps")
        s.mergeProps(s.props.v, t.props)
        carryMeThrough = s.props.v
        // zap the source props for the moment so it doesn't copy
        // how do you delete a property in scala? ideally,
        // we would just inform the real transferPropsTo to not
        // copy the "v" property since "v" is scalajs-react wrapper specific
        s.props.v = js.undefined
      }
    }
    val r = s.transferPropsTo(t).asInstanceOf[ReactComponentU[_, _, _]]
    // restore property to source
    if (carryMeThrough != js.undefined) {
      g.console.log("restoring source wrapped value so source will have original scala props object")
      s.props.v = carryMeThrough.asInstanceOf[js.Dynamic]
    }
    if (t.deserializeProps != js.undefined) {
      g.console.log("target has deserializeProps")
      if (t.props.v != js.undefined) {
        g.console.log("target has a props object, calling deserializeProps")
        // old target props + deserialized updates = new target props object
        val newProps = t.deserializeProps(t.props, t.props.v)
        // set the new props value to the new deserialized props
        t.props.v = newProps
        // Should we send an alert to the object that a new props was set?
      }
    }
    g.console.log("target properties after update: " + pobj(t.props))
    // we don't use the API about "setting" properties because all of this
    // flies under the radar just like in react
    r
  }

In the above code, t.deserializeProps is never defined on the target object since its unmounted and not "enhanced" yet.

In ReactComponentB.scala:

 def render(render: ScopeU => VDom): B4 =
 29        B4(render, undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined)
 30
 31      case class B4 private[ReactComponentB](
 32          __render: ScopeU => VDom
 33          , getDefaultProps: UndefOr[() => Props]
 34          , componentWillMount: UndefOr[ScopeU => Unit]
 35          , componentDidMount: UndefOr[ScopeM => Unit]
 36          , componentWillUnmount: UndefOr[ScopeM => Unit]
 37          , componentWillUpdate: UndefOr[(ScopeWU, Props, State) => Unit]
 38          , componentDidUpdate: UndefOr[(ScopeM, Props, State) => Unit]
 39          , componentWillReceiveProps: UndefOr[(ScopeM, Props) => Unit]
 40          , shouldComponentUpdate: UndefOr[(ScopeM, Props, State) => Boolean]
 41          , mergeProps: UndefOr[(Props, scalajs.js.Dynamic) => Unit]
 42          , deserializeProps: UndefOr[(scalajs.js.Dynamic, Props) => Props]
 43          ) {
 44
 45        def getDefaultProps(f: => Props): B4 = copy(getDefaultProps = () => f)
 46        def componentWillMount(f: ScopeU => Unit): B4 = copy(componentWillMount = f)
 47        def componentDidMount(f: ScopeM => Unit): B4 = copy(componentDidMount = f)
 48        def componentWillUnmount(f: ScopeM => Unit): B4 = copy(componentWillUnmount = f)
 49        def componentWillUpdate(f: (ScopeWU, Props, State) => Unit): B4 = copy(componentWillUpdate = f)
 50        def componentDidUpdate(f: (ScopeM, Props, State) => Unit): B4 = copy(componentDidUpdate = f)
 51        def componentWillReceiveProps(f: (ScopeM, Props) => Unit): B4 = copy(componentWillReceiveProps = f)
 52        def shouldComponentUpdate(f: (ScopeM, Props, State) => Boolean): B4 = copy(shouldComponentUpdate = f)
 53        def deserializeProps(f: (scalajs.js.Dynamic, Props) => Props): B4 = copy(deserializeProps = f)
 54        def mergeProps(f: (Props, scalajs.js.Dynamic) => Unit): B4 = copy(mergeProps = f)

There's also some housekeeping that is not totally clean in the above code, for example, mergeProps will leave the properties that the programmer has defined in the raw props object in addition to the scala object under props.v. Its not alot of duplication, but there needs to be a cleanup method or a way to determine what was added as part of mergeProps in the source object and clean up the props object. It's not alot of mess and probably not worth fixing right now, since setting the props happens so often.

@japgolly
Copy link
Owner

Interesting, it looks like another JS interop issue. Hey you know what would really help? If you could put one or a few tests together that show what you would expect to work, that'd be very helpful. Don't worry about making them pass, just showing that two simple components together with whatever properties generate wrong html because props aren't being propagated. Check out Test.scala; they're pretty quick and easy to write.

@aappddeevv
Copy link

I'll try. The issue seems to be that fb react seems to have written yet another version of a meta class layer that requires some programming around.

Here's a little snippet though based on trying to match react-bootstrap's 2 components for Panel and Label. This mirrors similar owner-child relationship that I need to create and interoperate with so I used these as a basis. There is some noise in the code due to debugging messages. The target is to get `testString``` to pass through in this case.

There is more clean up on the this.props object needed after using it as an integration point because the merged properties from the scale object that are merged into the this.props are still hanging around in the this.props object even after they have been deserialized by the child component. And I need to set the new properties object once it has been deserialized.

The other issues is where to put the transferPropsTo method. In your wrapper library formulation it either needs to be a separate function outside the ReactComponentB or it needs to a function inside it but then that may break some of the abstractions you have built.

The issue with the code below is that Label.deserializeProps never gets called because its not present on the label react component when the react component is in the unmounted state--its "mixed" in later in the react component's lifecycle.

case class LabelProps(
    bsClass: String = "label",
    bsStyle: String = "default",
    testString: String = "xxx")
  case class LabelState()
  class LabelBackend(val c: BackendScope[LabelProps, LabelState])

  val Label = ReactComponentB[LabelProps]("Label")
    .initialState(LabelState())
    .backend { s => new LabelBackend(s) }
    .render { (props, children, state, backend) =>
      g.console.log("rendering label and children: " + children + " values: " + pobj(children))
      React.Children.forEach(children, (child: VDom) => {
        g.console.log("child: " + pobj(child))
      })
      val cname = toBsClassSet(enhanceForBs(Map(
        "bsClass" -> props.bsClass,
        "bsStyle" -> props.bsStyle)))
      g.console.log("label with classes string: " + cname)
      val ch = tags.span(className := cname)(children).render
      transferPropsTo2(backend.c, ch)
      ch
    }
    .mergeProps { (sourceProps, targetProps) =>
      targetProps.bsClass = sourceProps.bsClass
      targetProps.bsStyle = sourceProps.bsStyle
      targetProps.testString = sourceProps.testString
    }
    .deserializeProps { (source, current) =>
      g.console.log("deserializeProps for label")
      val r = current.copy(testString = source.testString.asInstanceOf[String])
      g.console.log("new props: " + r)
      r
    }
    .create

and the Panel parent. This tests scala => scala ownership but if this works, the other scenarios should as well.

  case class PanelProps(
    header: Option[Seq[js.Any]] = None,
    footer: Option[Seq[js.Any]] = None,
    onClick: Option[Any => Unit] = None,
    key: Option[Any] = None,
    bsStyle: Option[String] = None,
    id: Option[String] = None,
    collapsable: Boolean = true,
    testString: String = "blah")

  case class PanelState(expanded: Boolean = true)

  class PanelBackend(val scope: BackendScope[PanelProps, PanelState]) {
    var isChanging: Boolean = false
    def handleSelect(e: SyntheticEvent[HTMLInputElement]): Unit = {
      g.console.log("clicked on panel header")
      for {
        func <- scope.props.onClick
        key <- scope.props.key
      } yield {
        isChanging = true
        func(key)
        isChanging = false
      }
      e.preventDefault()
      scope.setState(scope.state.copy(expanded = !scope.state.expanded))
    }
  }

  implicit def torc(o: Seq[js.Any]): ReactComponentU[_, _, _] = o.asInstanceOf[ReactComponentU[_, _, _]]
  implicit def torc(o: js.Any): ReactComponentU[_, _, _] = o.asInstanceOf[ReactComponentU[_, _, _]]

  val thePanel = Ref[HTMLElement]("thePanel")
  val theBody = Ref[HTMLElement]("theBody")

  val Panel = ReactComponentB[PanelProps]("Panel")
    .initialState(PanelState())
    .backend { s => new PanelBackend(s) }
    .render { (props, propsChildren, state, backend) =>

      def renderHeading() = {
        val newHeaders = if (props.collapsable) {
          div()(
            torc(props.header.get),
            renderAnchor(header))
        } else {
          div()(torc(props.header.get))
        }
        div(className := "panel-heading")(
          div(className := "panel-title")(
            newHeaders))
      }

      def renderFooter() = {
        div(className := "panel-footer")(torc(props.footer.get))
      }

      def renderBody() = {
        div(className := "panel-body", ref := theBody)(
          torc(propsChildren))
      }

      def renderCollapsableBody() = {
        div(className := "panel-collapse", id := props.id.getOrElse("blah"), ref := thePanel)(
          renderBody())
      }

      def renderCollapsableTitle(header: js.Any) = {
        div(className := "h4 panel-title")(renderAnchor(header))
      }

      def renderAnchor(header: js.Any) = {
        g.console.log("rendering anchor")
        val cname = if (state.expanded) "" else "collapsed"
        val hrf = "#" + props.id.getOrElse("")
        tags.a(className := cname, href := hrf, onclick ==> backend.handleSelect)(
          torc(header))
      }

      val component = div(className := "panel panel-default")(
        if (props.header.isDefined)
          renderHeading(),
        if (props.collapsable) renderCollapsableBody()
        else renderBody(),
        if (props.footer.isDefined)
          renderFooter()).render
        transferPropsTo2(backend.scope, component.asInstanceOf[js.Dynamic])
    }
    .mergeProps { (currentProps, target) =>
      g.console.log("mergeProps-scala side")
      target.testString = currentProps.testString + "-passed"
    }
    .deserializeProps { (source, currentProps) =>
      g.console.log("deserializeProps-scala side")
      currentProps.copy(testString = source.testString.asInstanceOf[String])
    }
    .componentWillReceiveProps { (scope, props) =>
      g.console.log("componentWillReceiveProps")
    }
    .shouldComponentUpdate((scope, props, state) => !scope.backend.isChanging)
    .create

This allows you to:

Panel(PanelProps(id=Some("testpanel"), testString="hello world"),
            Label(LabelProps(), span()("Big scary label"))

Notice that the span()("Big scary label"). It appears that type-wise, we can only stick components in as children, but fb react allows us to put anything in there but I had to wrap it as a span.

@aappddeevv
Copy link

There may be some easy way to get the transferred properties into the child object. If the transfer happens prior to mounting, perhaps one of the lifecycle functions can be used to absorb the transferred properties contained in the raw this.props object into the scala properties object. It's hard to tell how transferPropsTo intersects the lifecycle of a react component though.

After looking at some code, it also appears that the cleanup footprint that's needed after mergeProps is negligible. It's really not significantly worse than the raw JS fb react library. The cleanup occurs on the next setProps which may be frequently enough.

@japgolly
Copy link
Owner

Thanks for posting the code. I'm not a fan of the direction this is going, especially with the mutable state objects and side-effecting functions. Those things may be necessary for JS component interop but I'd like to get the Scala-side working nicely first.

Btw about having to wrap plain text in a span, wrap it in raw() instead. It's a Scalatags thing. So raw("Big scary label").

Also tests just got easier to write so have another look. If I have tiny tests this will get implemented much faster. Cheers.

@japgolly
Copy link
Owner

Also for the className := toBsClassSet(enhanceForBs(... part, I recently added classSet as part of the library. Check the readme or Tests to see how it's used. Hope that helps.

@japgolly
Copy link
Owner

One more; where you've got header: Option[Seq[js.Any]] etc, I think you want header: Option[VDom]. Better type-safety and you don't need hacky torc functions.

@aappddeevv
Copy link

I'll look into the torc changes. I hacked the types just to get it to compile since I was only interested in trying an extended example for working with children and transferPropertiesTo.

The children processing is important even for pure scala hierarchies of more than 3+ layers deep (like Panel -> Label -> display content). I hit the problem almost immediately when trying to build a screen and searched for a solution. I then found the bootstrap react library and realized that I needed children processing and properties transfer because that's how they solved similar issues. I needed this for styles and some info needed for rendering (a set of flags that switch some fields from editable to just display).

It's quite possible the new "context" feature of fb react could be used but that's also not in scalajs-react either. You would need to add the ability to find "objects" in the component (this.context) and the ability to specify the name and types of context values. Similar in concept to this.props, It merges the current context into the child context and makes it available on the child through this.context. I think context is best kept as a js object (perhaps map it to a js.Dictionary new in 0.5?) since there are many problems around property transfers in scalajs-react right now. Your API based on passing in only what's needed to the react component function may start feeling constraining since you have state, props, contexts and who knows what else, popping in to help components perform their processing.

I think the only difference between properties and context values is that properties can be set by anything using setProps but context is only "set" by the owner.

@aappddeevv
Copy link

I've been looking through the react code some more and looking at extensions through mixins. I'm not convinced now that a purely function approach will work when you consider mixins which can add data and methods to instances on the js side. While I don't disagree with your comment on mutability and other things, it's not clear you can make a scala side that can fully interop with the react js side without bending the purer rules a bit or making it unnecessarily complicated. Even react js bends the rules and is not really react in a more pure sense. Maybe type classes and abstract data types are going to enter the picture more.

@japgolly
Copy link
Owner

I'm going to have to park this for a while and come back to it later. If you could submit any tests (they're super easy to write now) that demonstrate what this is lacking that would be awesome.

@japgolly japgolly changed the title React Bootstrap Property inheritance (Scala→Scala) Jul 21, 2014
@japgolly
Copy link
Owner

Seeing as this discussion has jumped around a bit, I'm allocating the rest of its lifespan to "Property inheritance (Scala→Scala)". For JS ↔ JS/Scala property inheritance, or other JS-interop issues, let's use #8.

@japgolly
Copy link
Owner

Property inheritance (Scala→Scala) isn't something I need to address. Scala itself provides enough for that and #8 (JS-interop) is the important one.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants