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

Experiment: try letting CompositeComponent have an array of _renderedComponents #10

Open
thomasboyt opened this issue Jun 3, 2015 · 5 comments

Comments

@thomasboyt
Copy link
Owner

I'm concerned that the whole ReactDOMFragment idea was bad from the start, given the trouble I've had figuring out how to store and update the state of nodeCounts/Indexes. This won't solve #7, of course, but would at least make it easier to make a simple "child -> parent" lookup without going through the mess that is ReactDOMFragment...

The main difficulty here would then be turning

<Component>
  <frag>
    <li>foo</li>
    <li>bar</li>
  </frag>
</Component>

into:

<Component>
  <li>foo</li>
  <li>bar</li>
</Component>

I wonder if making frag turn into an array under the hood would be the best way to do it...

Oh, and I guess this would mean CompositeComponent now also has to be a ReactMultiChild-using component. That could be... interesting...

@thomasboyt
Copy link
Owner Author

hm, so here's an issue with this plan. the following:

<Component>
  <frag>
    {[<li>foo</li>,
      <li>baz</li>]}
    <li>bar</li>
  </frag>
</Component>

becomes:

<Component>
  {[<li>foo</li>,
    <li>baz</li>]}
  <li>bar</li>
</Component>

and while it may be possible to allow components to render multiple root components, expanding arrays into multiple root components seems potentially more complicated to maintain.

@thomasboyt
Copy link
Owner Author

If fragments are flattened into children, the way root nodes work will need to be overhauled to support multiple nodes. Right now this:

<Component>
  <frag>
    <li>foo</li>
  </frag>
  <frag>
    <li>bar</li>
  </frag>
  <li>baz</li>
</Component>

ends up as:

<Component id=".0">
  <li id=".0.0">foo</li>
  <li id=".0.0">bar</li>
  <li id=".0.2">baz</li>
</Component>

when it should be something to the effect of:

<Component id=".0:0">
  <li id=".0:0.0:0">foo</li>
  <li id=".0:0.0:1">bar</li>
  <li id=".0:0.1:0">bar</li>
</Component>

where 0:0 means composite_id:node. might be possible to shorten x:0 to just x for readability, too...

@thomasboyt
Copy link
Owner Author

I think that, because of how SEPARATOR is implemented in ReactInstanceHandles, changing this format actually isn't all that bad - a key that says 0.0:0.0 should be able to be handled by any code that was able to handle 0.0. The only thing that needs to change is the actual generation of the keys, which I believe happens in flattenChildren (through traverseAllChildren).

@thomasboyt
Copy link
Owner Author

huh, fun fact: : is already used a subseparator for arrays. I feel like this should be able to use the same separator for fragments, but have to think over it for a bit...

@thomasboyt
Copy link
Owner Author

current major issue here is that reconciliation isn't easy. it'd require either:

a) basically reimplementing ReactMultiChild in ReactCompositeComponent, or
b) mixing in ReactMultiChild to ReactCompositeComponent (which is likely the better option), which will require some reshuffling to prevent circular dependencies

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

1 participant