Workiva/react-dart

Support nested Lists of children

greglittlefield-wf opened this issue · 7 comments

In JS, React renders children that consist of nested arrays by flattening them:

// JS, nested
A({},
  B({}),
  [
    C({}),
    C({}),
    [
      D({}),
    ]
  ],
  B({})
)
// flattened rendered children of A
[B, C, C, D, B]

This doesn't work with nested Dart Lists, but it does work if JsArrays are specified instead:

// Dart, nested Lists
A({}, [
  B({}),
  // Nested Dart List (DartObject from React's perspective)
  [
    C({}),
    C({}),
    // Nested Dart List
    [
      D({}),
    ]
  ],
  B({})
])
// flattened rendered children of A (nested Lists don't appear)
[B, B]


// Dart, nested JSArrays
A({}, [
  B({}),
  new JsArray.from([
    C({}),
    C({}),
    new JsArray.from([
      D({}),
    ])
  ]),
  B({})
])
// flattened rendered children of A (nested converted Lists appear!)
[B, C, C, D, B]

Perhaps react-dart should deep-convert nested Lists in children to JsArrays. Thoughts?

Yeah, that sounds reasonable. I will be really grateful for PR!:)

Cool! I'll have a PR within a couple days, then.

Cool! I'll have a PR within a couple days, then.
Way to overestimate again, @greglittlefield-wf...

Haha in all seriousness, I did get an implementation for this hacked together over the weekend that I'm pretty happy with.

I don't currently have anything clean enough to commit, though, and I still need to play with it a little to optimize the performance.

I'll probably have it finalized and ready for PR sometime this week.

So, I realized that my implementation was missing the key remapping that was present in React.Children.Map.

Any objections to including similar utilities that properly deal with nested children, like those present in React JS? https://facebook.github.io/react/docs/top-level-api.html#react.children

(Also, sorry, kind of forgot about this.)

@greglittlefield-wf does the work in #88 take care of this?

@aaronlademann-wf Didn't think of that; yup, it does! This issue can be closed when that merges.

@trentgrover-wf this can be closed.