Add reference to parent instance for component instances

[syranide]

@chenglou I'm pretty sure you want something like this for your event overhaul right? (to get rid of ID hacking)

After my short discussion with @sebmarkbage in #1922, I realized that I could implement a compromise of #1570. Where instead of removing data-reactid entirely, we can shorten it to just data-react (with no value) until the refactor is done and we have had a chance to evaluate and decide on the most preferable behavior.

The advantage of the compromise is that it will be more isolated and will work identically to the current lazy implementation. So server-generated markup should be considerably smaller (compression will remove any data-react overhead), we also gain some of the performance benefit of the shorter markup.

This is probably a hot topic, but I believe this is inevitable if we want to get rid of ID hacking and concatenated IDs (which I'm positive we do want to get rid of).

[syranide]

We want to keep React core explicitly independent of ReactDOM related code, but only ReactDOMComponent is supposedly interested in keeping track of its ReactDOMComponent parent (for performance reasons we want to avoid traversing through non-ReactDOMComponents). As it will only be used by our ReactMount and our synthetic event system.

However, since mountComponent comes from React core, we can't extended it with anything specific to ReactDOM.

I see two solutions:

1. Extend mountComponent with a parents-parameter, a push/pop array of all parent components that each ReactComponent-specialization can do as they please with, we assume nothing (@sebmarkbage didn't seem pleased with this :)).
2. Extend mountComponent with a parentNode-parameter, add ReactNodeComponent to core which ReactDOMComponent would mixin. ReactNodeComponent would store this._parentNode which references the closest ReactNodeComponent.

[sebmarkbage]

2 sounds good. ReactDOMComponent code will eventually move behind the serialization/diff stage.

[syranide]

@sebmarkbage Apparently I oversimplified reality quite a bit.

It's fine to store _parentNode in only ReactNodeComponent for mountComponent. However, updateComponent for e.g. ReactCompositeComponents requires that _parentNode is known when it calls mountComponent, which should mean that _parentNode has to be stored by ReactComponent for all components...

...unless we store _parent for all components and _parentNode only for ReactNodeComponent, when updateComponent wants _parentNode it would traverse up _parent until it finds a non-null _parentNode. The benefit is that would we track _parent, but it's also a cost if we don't have any use for it (I'm not suggesting we do this).

(System architecture really isn't my strongest side, so perhaps I'm missing something obvious)

[syranide]

I realize I probably don't understand the React architecture well enough to be in there.

I've updated the PR, it uses getClosestNode() (overloaded by ReactNode) to do its thing. Doesn't seem great, but necessary unless we can guarantee that ReactMultiChild is exclusive to ReactNode (and never used by ReactCompositeComponent). Looking at the implementation of ReactMultiChild that is probably true though. However, getClosestNode() could still end up being necessary, unless this is solved some other way.

Kind of unrelated, but related:

My hope was to also move _rootNodeID to ReactNode only and rely on getClosestNode() instead, but _rootNodeID references the closest descendant node, and does so before that node actually exists (courtesy of ID magic), which is kind of hacky. This setup also depends on ReactCompositeComponent only being able to have a single child... I imagine that we might want to support multiple children in the future? (or maybe not because it has some weird side-effects?)

ReactBrowserComponentMixin is injected into ReactComponent and builds on the same assumptions/hacky code as above. Theoretically, a separate mixin should be injected into ReactComponent that exports getDOMNodes() instead, or getDOMNode() without ID magic.

It seems that ReactComponent.BackendIDOperations should also move to ReactNode, but ReactCompositeComponent also uses it with the assumption that a composite component only can have a single child, and the same ID magic again.

I don't know what to make of this. :)

[sebmarkbage]

We should use a different terminology in the internals for referring to these nodes so that we don't overload it's meaning with the proposed public ReactNode interface.

My plan for ReactCompositeComponent was actually for it to reach out to the _parentNode to do it's replacement so that it could handle changes in fragments too. That would make it needed on all nodes. Jordan may have a different idea though. We should coordinate on this because there are so many overlapping requirements here.

[syranide]

We should coordinate on this because there are so many overlapping requirements here.

Yeah I kind of realized that too, leaving it for you guys (again :)), unless I can make myself useful somehow.

PS. Closing because this is not really an "issue" and it will happen eventually anyway.

[sebmarkbage]

@syranide I'll be in Stockholm and Örebro in September. We should have a hackathon. :)

[syranide]

@sebmarkbage Haha, that would be awesome ;)