thomasboyt / react

React is a JavaScript library for building user interfaces. It's declarative, efficient, and extremely flexible. What's more, it works with the libraries and frameworks that you already know.
http://facebook.github.io/react/
Other
0 stars 1 forks source link

Update ReactMultiChild._updateChildren to be fragment-aware #1

Closed thomasboyt closed 9 years ago

thomasboyt commented 9 years ago

The update children algo uses indexes to decide what gets removed/moved/etc. These indexes are off for adjacent fragments:

<frag key="1">
  <div>foo 1</div>
  <div>bar 1</div>
</frag>
<frag key="2'>
  <div>foo 2</div>
  <div>bar 2</div>
</frag>

trying to remove the 2 fragment will result in the following DOM:

<div>foo 1</div>
<div>foo 2</div>
<div>bar 2</div>

This is because it tries to remove the DOM element at index 1, thinking it's the fragment, while instead it needs to remove both children.

thomasboyt commented 9 years ago

I actually think in this case it's doing the right thing, it's the DOM operations that need to be updated to treat fragments specially... hm.

thomasboyt commented 9 years ago

Actually, it does make sense for _updateChildren to have this logic, since it needs to generate instructions that can be understood by the "true hierarchy" instead of the internal one

thomasboyt commented 9 years ago

Specifically, what I'm trying to solve is:

remove_child(component):
  if component.type == fragment:
    for child in component:
      remove(child)
    return
thomasboyt commented 9 years ago

Ah, here's the real problem: component isn't the fragment, it's the component that renders the fragment. What we'd really need is a way to look up its element and see if it's a fragment, and if so generate removals for its own children.

This may be slightly impossible, given the current logic.

thomasboyt commented 9 years ago

Further investigation it may be easiest to set up a register of which component IDs are fragments, then look those up during the updateChildren cycle. If it encountered them, it could theoretically not generate a removal instruction for the parent... Would also need to have a way to then get a list of child IDs... hm.

thomasboyt commented 9 years ago

Something worth thinking about at this point is that the existing index-based operations completely break down the moment you have multiple adjacent fragments. Obviously, given the structure in the first example, the ideal operations to generate would be:

[{type: REMOVE_NODE, fromIndex: 2},
{type: REMOVE_NODE, fromIndex: 3}]

However, calculating those latter indices would require some sort of internal cache of offsets, like,

{
  ".0.0.$1": {
    ".0.0.$1.0": 0,
    ".0.0.$1.1": 2
  }
}

This could be quite costly/tricky to maintain (especially if fragments can be nested).