[WIP][Fiber] React Native Fiber

[edvinerikson]

• Fix Invalid layout for (23)<RCTView: 0x10053ac60; reactTag: 23; frame = (3 207; 272 68); layer = <CALayer: 0x17002c520>>. position: {139, nan}. bounds: {{0, 0}, {272, nan}}
  Seems to be a issue not related to Fiber. Having the issue when running RN master with old renderer
• Cleanup instances on native side
• remove cached nodes from component tree
• devtools (?)
• fix removedChildren count (%tu) was not what we expected (%tu)
• add insertBefore 8d558bd
• add container info d813f1e
• move stack/shared modules into it's own folders. 7694ea6
• clear old children when updating container 60cc310 0700a88
• support going from multi-child to single-child and opposite 257cdbf
• append full child in children list. (?) might be useful if we want to remove the tag model later on.
• split some parts into a ReactNativeFiberComponent like in DOM. (?)
• try to remove children list and use something provided by Fiber.
• make sure tests pass
• add tests
• remove temp build script

[edvinerikson]

Maybe a rewrite of the bridge would be good? E.g to make children operations operate on the tags / instances instead of on the indices.

UIManager.appendChild(
  parentTag,
  childTag
);

UIManager.removeChild(parentTag, childTag); // does not remove from registry
UIManager.insertBefore(parentTag, beforeTag, childTag);


// current api
UIManager.manageChildren(
  parentTag,
  [] // move from indices
  [] // move to indices
  [] // addReactTags
  [] // add tags at indices
  [] // remove view at indices. this also removes from registry
)

Not sure it's possible to send JSObjectRefs if we want to replace the tag model with that though.

[sebmarkbage]

Unfortunately I will probably not have much time to review this in the coming week since that will take focus away from testing the DOM implementation.

It is also unlikely that we'll be able to rewrite the bridge in the initial test. It will simply be too much to test at the same time. So we'll need to first do an implementation that corresponds to the old bridge and then iterate on the bridge later. For example, JSObjectRefs would destroy the debugger so we'd need a whole new debugger strategy. There's a lot of possibilities there.

The reason we treat string/number children special is that the host can choose to do an optimized path doesn't require extra backing stores inside React for those cases since you know that it will always be the only child of the parent you don't need React to keep track of them for you.

There are issues switching between those modes atm though. See #8331

[edvinerikson]

Unfortunately I will probably not have much time to review this in the coming week since that will take focus away from testing the DOM implementation.

I can see that. No hurry. :)

It is also unlikely that we'll be able to rewrite the bridge in the initial test. It will simply be too much to test at the same time. So we'll need to first do an implementation that corresponds to the old bridge and then iterate on the bridge later. For example, JSObjectRefs would destroy the debugger so we'd need a whole new debugger strategy. There's a lot of possibilities there.

Yeah, I agree. what debugger? I've been looking for something to debug the bridge but couldn't find anything. I used the "view hierarchy" tool in xcode instead. Is there something else?

The reason we treat string/number children special is that the host can choose to do an optimized path doesn't require extra backing stores inside React for those cases since you know that it will always be the only child of the parent you don't need React to keep track of them for you.

This explains a lot of confusion! Thank you!

I will continue to experiment with this and see how I can improve it.

[sebmarkbage]

We have the previous and next index available on the Fibers when a move happens. We could that information along to the host in insertBefore and appendChild.

[edvinerikson]

I have now re-written all of it again trying to follow patterns used in the DOM renderer.
I tried to split the work in separate commits so it's a little bit easier to follow. I think I got all event handling to work properly now as well.

Still a lot of stuff left such as devtools integration, instance cleanup, insertBefore and probably a lot of other things as well..

We have the previous and next index available on the Fibers when a move happens. We could that information along to the host in insertBefore and appendChild.

Yeah that could work.

[edvinerikson]

Also on the topic of text reset. shouldn't this work: https://github.com/edvinerikson/react/blob/c252161d7653d6945800a95f7b08ee8f34451bba/src/renderers/native/fiber/ReactNativeFiber.js#L123-L132 ?
IMO #8331 shouldn't be needed. what am I missing?

[sebmarkbage]

Actually the index thing might not work when fragments are enabled.

[sebmarkbage]

The issues is that appendChild/insertBefore gets called before commitUpdate.

[edvinerikson]

The issues is that appendChild/insertBefore gets called before commitUpdate.

alright, I guess I can still handle that by memoizing the tag and search for it's index.

Actually the index thing might not work when fragments are enabled.

I will look into that, think it will be nice if we can get rid of the virtual list.

[edvinerikson]

when removing a instance from the ReactNativeComponentTree cache is https://github.com/facebook/react/blob/master/src/renderers/dom/fiber/ReactDOMFiber.js#L142-L144 that the correct place to do it?

[sophiebits]

@sebmarkbage We don't have the index within the host handy, right? Just the parent host/composite/fragment.

[sophiebits]

@edvinerikson Any reason you closed this?

[edvinerikson]

I heard that Brian have been working on this internally and I haven't gotten the time to update it for a while. I can re-open if we still want to continue with this approach.

…

On Sat, 10 Dec 2016 at 22:05, Ben Alpert ***@***.***> wrote: @edvinerikson <https://github.com/edvinerikson> Any reason you closed this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8361 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADtdjLEaPpdM4DLmyRdtCQNtN6mUq-Ciks5rGxQxgaJpZM4K3lXa> .

[sebmarkbage]

@spicyj We don't have the index handy, no. I thought for a while that we could reuse .index if we didn't allow fragments. However, that's not sufficient because leading null children will bump the .index but won't exist in the host.