Remove dormant createBatch experiment #17035
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In a hybrid React app with multiple roots, `createBatch` is used to coordinate an update to a root with its imperative container. We've pivoted away from multi-root, hybrid React apps for now to focus on single root apps. This PR removes the API from the codebase. It's possible we'll add back some version of this feature in the future.
|
Are there any complexities in the workloop that can be simplified by removing this feature? |
acdlite
commented
Oct 7, 2019
| @@ -418,64 +224,24 @@ function ReactRoot(container: DOMContainer, options: void | RootOptions) { | |||
| ReactRoot.prototype.render = ReactSyncRoot.prototype.render = function( | |||
| children: ReactNodeList, | |||
| callback: ?() => mixed, | |||
| ): Work { | |||
There was a problem hiding this comment.
@sebmarkbage I also changed this to return nothing instead of a thenable. It still accepts a callback. I actually forgot that we supported both forms... I don't think this was intentional? Let's choose one, or neither. I don't have a preference; I went with the callback for now because it's the simpler implementation.
Details of bundled changes.Comparing: cd1b167...e4b21be react-dom
react-art
react-native-renderer
react-test-renderer
react-reconciler
react-noop-renderer
|
sebmarkbage
approved these changes
Oct 7, 2019
Most of them were in ReactFiberReconciler.js and ReactDOM.js. Though I now see I can also delete |
|
I believe some of the exports from ReactFiberReconciler aren’t used by any renderer so we can delete those too while you’re at it. Although hopefully they are dce in the build already. |
|
@sebmarkbage Any reason |
|
It’s called by a renderer so needs to be exported by the reconciler since that’s the public api. But you can in-line it one way or another. It’s in the reconciler because it also does changes to the retry time. Also the other hydration functions are there. Seemed like we didn’t want to bloat the work loop with too many concerns. |
|
You can maybe simplify flushRoot though to something simpler. |
|
Ok I'll rename it and remove the invariant |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In a hybrid React app with multiple roots,
createBatchis used to coordinate an update to a root with its imperative container.We've pivoted away from multi-root, hybrid React apps for now to focus on single root apps.
This PR removes the API from the codebase. It's possible we'll add back some version of this feature in the future.