Proposal: alternative method for handling static vs dynamically-generated child element lists

Hello everyone! I am just getting into purescript, and I am looking forward to having another functional language in my life.

Trying out purescript-react-native, I immediately ran into problems with React complaining about missing key props in child elements. I see from the code in purescript-react and from the discussion in #53 that purescript-react has addressed this issue by providing the modules React.DOM and React.DOM.Dynamic to separate components that take statically-known children vs components that take dynamically-computed child lists. This is nice, but there are two potential issues:

I predict that I will forget to use React.DOM.Dynamic functions when I dynamically generate child lists, and I will forget to put key props on those child elements. When this happens I will not get warnings from React due to the functions in React.DOM converting child lists to spread arguments when calling React.createElement.
The distinction between components that take static vs dynamic child lists prevents mixing static and dynamic children in a single child list - which is something that React supports.

I had a thought that these issues could be addressed by emulating lucid's trick of abusing do notation to sequence Html elements.

(As I mentioned, I am a newcomer to purescript. I apologize if this is well-trodden ground, or if this post is otherwise unhelpful.)

My thinking is that instead of using the raw ReactElement type in purescript, one could use a wrapper that represents arrays of ReactElement values. Elements in the array are tagged, so that static children are represented by plain ReactElement values, while dynamically-generated children are represented as a nested array.

data TaggedElement = StaticElement ReactElement
                   | DynamicElements (Array ReactElement)

data ReactElementsImpl a = ReactElementsImpl (Array TaggedElement) a

type ReactElements = ReactElementsImpl Unit

With this type only one code path is required for invoking the javascript React.createElement function:

function createElement(class_) {
  return function(props) {
    return function(children) {
      var unwrappedChildren = children.map(c => c.value0);
      return React.createElement.apply(React, [class_, props].concat(unwrappedChildren));
    };
  };
}

Child element lists are unconditionally passed as spread arguments - but because dynamically generated child lists are in nested arrays, those lists map to array arguments to createElement, which causes React to infer that children in those arrays are dynamically-generated.

The ReactElements type can be made user-friendly by avoiding exposing the TaggedElement type or its constructors directly to users. Instead, library-defined components are given in the form of ReactElements values, and these are composed into child lists using a Bind instance.

instance functorReactElementsImpl :: Functor ReactElementsImpl where
  map f (ReactElementsImpl es x) = ReactElementsImpl es (f x)

instance applyReactElementsImpl :: Apply ReactElementsImpl where
  apply (ReactElementsImpl as f) (ReactElementsImpl bs x) = ReactElementsImpl (as <> bs) (f x)

instance bindReactElementsImpl :: Bind ReactElementsImpl where
  bind (ReactElementsImpl es x) f =
    case f x of
      ReactElementsImpl es' y -> ReactElementsImpl (es <> es') y

(This could be a bit simpler with an applicative-do feature ;)

A user could write code that looks like this:

render _ = return $
  div' $ do
    span' (text "This is an example")
    span' (text "of a construct that `do` notation was arguably not intended for")

When the user wants to dynamically generate an array of elements, they would use a smart constructor elements to transform an array of ReactElements values into a single value.

elements :: Array ReactElements -> ReactElements
elements es = ReactElementsImpl [DynamicElements (flat es)] unit
  where
    flat reArray = do
      reactElements <- reArray
      case reactElements of
        ReactElementsImpl taggedElements _ -> do
          taggedElement <- taggedElements
          case taggedElement of
            StaticElement   rawElem  -> [rawElem]
            DynamicElements rawElems -> rawElems

The elements implementation flattens the input elements array so that there are always exactly two levels of nesting in ReactElements values.

Using elements, a user can mix static and dynamic children as desired:

render _ = return $
  div' $ do
    span' (text "This is an example")
    span' (text "of mixed strict and dynamic child elements")
    ul' $ do
      li' (text "first item")
      elements $ (\n -> li' (text ("list item #" ++ show n))) <$> 1..10
      li' (text "last item")

This opens up another possibility of implementing the elements constructor in such a way as to statically enforce the requirement that dynamically-generated elements should have a key prop:

elements :: Array { key :: String, element :: ReactElements } -> ReactElements

That would just require a function to modify a given ReactElement to add the key property after-the-fact.

I have done just enough experimentation with this idea to verify that the code above type-checks. I wanted to get some feedback to see if people like this idea before trying for a working implementation. So, what do you all think?

Thanks for your proposal and digging into the static vs. dynamic behaviour. I am open to ideas on this. However, I think I need to spend a bit more time with what you're suggesting above. Do you happen to have a branch with these changes available to try out?

I don't have a branch. I just have these files that I used to work out the types:

https://gist.github.com/hallettj/dac3194b0a12bf44f94c89da386db12c

That obviously does not render anything yet. I could put together a working implementation sometime in the near future.

Thanks for the gist. I was just curious to take more of a look on how things would work with this change.

I have branches ready for purescript-react and for purescript-react-dom. The branches are named proposal/do-notation in both repos.

https://github.com/hallettj/purescript-react/tree/proposal/do-notation
https://github.com/hallettj/purescript-react-dom/tree/proposal/do-notation

There are still foreign functions in the React module that need to be updated for the changes to types. But there is enough code in place to experiment with the proposed API.

Instead of adding a ReactElements type, I redefined ReactElement to serve that purpose, and renamed the original ReactElement type to ReactElementRaw.

I also added a Partial constraint to functions in ReactDOM. This due to an issue that the new ReactElement type brings up: it wraps an array of elements, but React's render and associated functions require a single element as an argument.

Here is a short usage example:

module Main where

import Prelude (Unit(), show, (++), (<$>), ($), bind)

import Control.Monad.Eff (Eff)
import Control.Monad.Eff.Console (CONSOLE, log)
import Data.Array ((..))

import React (ReactElement, elements)
import React.DOM (div', li, li', span', ul', text)
import React.DOM.Props (key)
import ReactDOM (renderToString)

markup :: ReactElement
markup =
  div' $ do
    span' (text "This is an example")
    span' (text "of mixed strict and dynamic child elements")
    ul' $ do
      li' (text "first item")
      elements $ (\n -> li [key ("li" ++ show n)] (text ("list item #" ++ show n))) <$> 1..10
      li' (text "last item")


main :: forall e. Partial => Eff (console :: CONSOLE | e) Unit
main = do
  log (renderToString markup)

I'd like to sum up what I see as the pros and cons of this API change. The cons:

components imported via FFI must be wrapped. (wrapReactElementRaw does this, but needs a better name).
render and associated functions become partial
more array allocations, more copying, more function invocations due to use of bind

and the pros:

static enforcement of best practices for dynamically-computed child element lists
possibility of mixing dynamically-computed child elements lists with statically known children
I think that do notation is kinda nice for declaring markup

Despite my love of lucid -- heck, we're using lucid in production for our less dynamic pages -- I'm strongly against this change. To be frank, I will maintain a separate library if these changes are implemented. (Which is completely fine btw, as we're heavily invested in the library.)

As for the cons... We use a ton of imported components; for us, do notation wouldn't trump convenient third-party component FFI. Further, I'd prefer it if render et al. were not partial functions.

In my opinion, the 'pros' really aren't worth the candle. I don't believe that "static enforcement of best practices" belongs in a low-level wrapper, nor do I believe that do notation makes the DSL any nicer.

@jasonzoladz

@jasonzoladz: Thanks; this is the kind of feedback I was hoping for.

@hallettj

@hallettj: I hope it didn't come off as harsh. And, for the record, my opinion here likely means little. It's Eric and Phil you'd need to convince. I'm just coming at this from a very practical place (the Snoyman-side of the Gonzalez/Snoyman divide, if you will).

Aside: I do recognize that your Galois-creds indicate that you are at least 3x the programmer that I am. ;)

Navigation Menu

Repository navigation

Proposal: alternative method for handling static vs dynamically-generated child element lists #74

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Proposal: alternative method for handling static vs dynamically-generated child element lists #74

Description

Activity

ethul commented on Apr 8, 2016

hallettj commented on Apr 8, 2016

ethul commented on Apr 9, 2016

hallettj commented on Apr 10, 2016

jasonzoladz commented on Apr 10, 2016

hallettj commented on Apr 10, 2016

jasonzoladz commented on Apr 10, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions