-
Notifications
You must be signed in to change notification settings - Fork 47.3k
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
Injection of ReactCompositeComponent
implementation
#4012
Comments
My intuition is: now that 0.14 is splitting renderers out from core, it should be easier/better to just have a separate renderer for server side. A substantial portion of the logic can be simplified/removed, streaming can be implemented, skipping component instantiation becomes a possibility (ok, maybe that one is hard/questionable), etc. Things that are common to client and server can be moved into shared, and things that are different can be forked. I tend to like that strategy better than injection/factories, because I find it more difficult to reason about the flow of control with the latter patterns. That said, I'm curious what @sebmarkbage thinks. |
@jimfb The problem is when you want to fork something in the middle and don't want to fork everything above it. We do want a system for injections but I would like it to become statically resolved. E.g. through overrides of requires. Then the shared code would be copied into each renderer build (i.e. not shared on npm), then minified and dead-code eliminated as a unit. |
@mridgway The current set up is really ad hoc and messy as part of incremental refactoring. :) So here's the plan that I'm shooting for... I would for renderers to be a two layer system:
This should make it trivial to target a new imperative API by just plugging in the environment specific parts into the generic reconciler part. There is currently a necessary cycle between ReactCompositeComponent and ReactDOMComponent because ReactDOMComponent causes a recursion which may lead back to ReactCompositeComponent. We solve this using a dynamic hacky injection workaround, which is unfortunate for static analysis and VM optimization reasons. However, that recursion is an implementation detail that we would like to get out of the environment specific logic. I.e. the DOM specific code shouldn't do any recursion. That should be part of the generic React reconciler. That way the entire cycle can move into ReactReconciler as a unit. No more module cycles. |
I wanted to circle back on this as I think it blocks some other changes that I wanted to look into (streaming and async). I started implementing your suggestion of moving the recursion up to The unfortunate thing is that the Any other thoughts on the best way to decouple the |
In researching a separate server implementation, I have found that being able to inject different component types in to
instantiateReactComponent
allows me to bypass much of the reconciliation logic that is not needed on the server. ViaReactNativeComponent
,instantiateReactComponent
most of the types are injectable with the exception ofReactCompositeComponent
.I've also noticed that a lot of the component injection is spread in different two files:
ReactEmptyComponent
andReactNativeComponent
. Neither of these seem like a good place to add CompositeComponent injection.These injection containers also contain a default implementation for many of their methods which could bloat package sizes if the defaults are overridden as they would likely be for the server.
I would propose to have a
ReactComponentFactory
that contains theinstantiateReactComponent
method and provides the injection for each type of component. Defaults implementations would be provided only byReactDefaultInjection
, which would likely also have a derivativeReactServerDefaultInject
.This would also allow removal of this circular dependency wrapper: https://github.com/facebook/react/blob/master/src/renderers/shared/reconciler/instantiateReactComponent.js#L24
The text was updated successfully, but these errors were encountered: