Fork React Native render into an "RT" renderer#11072
Fork React Native render into an "RT" renderer#11072sebmarkbage merged 1 commit intofacebook:masterfrom
Conversation
This is an experimental new protocol for some experiments we want to play with. To make that easier, I'm just going to fork it. This experiment won't use the event system so I by-pass it and just invoke functions on the props object for now. I also fork the UIManager into a new RTManager.
| declare function prependChild( | ||
| childTag : number, | ||
| beforeTag : number, | ||
| ) : void; |
There was a problem hiding this comment.
No parentTag? (Is this inferred from the beforeTag?)
| "react-dom.development.js (UMD_DEV)": { | ||
| "size": 613875, | ||
| "gzip": 141842 | ||
| "size": 614249, |
There was a problem hiding this comment.
Should we update this file or leave it until the next release we build?
There was a problem hiding this comment.
Doesn't really matter. :-)
| let root = roots.get(containerTag); | ||
|
|
||
| if (!root) { | ||
| // TODO (bvaughn): If we decide to keep the wrapper component, |
| internalInstanceHandle: Object, | ||
| ): void { | ||
| updateFiberProps(instance, newProps); | ||
| RTManager.updateNode(instance, newProps); |
There was a problem hiding this comment.
Is it okay to always make update calls (even if there are no changes)? We added a check to avoid no-op bridge calls for RN b'c of the expense.
There was a problem hiding this comment.
We don't have any view config and no structural info at this point. We could maybe do a shallow clone. We probably need to do a pass anyway since the props will contain function pointers that are not serializable. Not sure what we'll do here yet.
| const tag = ReactNativeRTTagHandles.allocateTag(); | ||
| RTManager.createNode(tag, type, props); | ||
| precacheFiberNode(internalInstanceHandle, tag); | ||
| updateFiberProps(tag, props); |
There was a problem hiding this comment.
We update props in commitUpdate and createInstance. Is that odd? Should it be in commitUpdate and commitMount instead? (I realize you're doing the same thing React Native does
but I'm questioning that now too.)
There was a problem hiding this comment.
That is updating the internal cache slot that keeps the "current event listeners". Really should only need to update when event listeners change but it's cheaper to just update it than checking. During initial mount we just reuse the same method since it does exactly the same.
There was a problem hiding this comment.
My question was more about the fact that we update this during commit phase in one place and complete phase in another. I was asking if we should wait to update in commit phase in both places.
There was a problem hiding this comment.
Right. We also do that with any other props since it's better to start work during the render phase if we can do as much work as possible prepared before we commit so that the commit is fast. During the initial mount phase nothing is in the "tree" yet so we won't get any events. Just like we can update props on newly created nodes without waiting for the commit phase.
The only quirk here is that this set will leak if it doesn't commit, which is a more general problem.
There was a problem hiding this comment.
The only quirk here is that this set will leak if it doesn't commit, which is a more general problem.
Thanks for clarifying. This is more what I was concerned about ^ but if this is a conscious decision on your part, then I'll defer to your judgement. 😄
| * Why: Because `dangerouslyReplaceNodeWithMarkupByID` relies on being able to | ||
| * unmount a component with a `rootNodeID`, then mount a new one in its place, | ||
| */ | ||
| var INITIAL_TAG_COUNT = 1; |
There was a problem hiding this comment.
The reason for this variable is not obvious to me.
(I realize that it also exists in ReactNativeTagHandles.)
There was a problem hiding this comment.
Me neither. I think it's because 0 is the default or something.
4 participants
This is an experimental new protocol for some experiments we want to play with. To make that easier, I'm just going to fork the React Native renderer.
This experiment won't use the event system so I by-pass it and just invoke functions on the props object for now.
I also fork the UIManager into a new RTManager.