Remove Reconciler forking #25773
Remove Reconciler forking #25773
Conversation
|
Rather than removing all the infra around it, can you remove just the forked files and keep the rest of the infra in place so we can enable it again in the future? Although we're not using it currently, it's been extremely helpful several times in the past. I used it over the summer to land a complex refactor. |
| @@ -30,7 +30,6 @@ export { | |||
| unstable_batchedUpdates, | |||
| unstable_createEventHandle, | |||
| unstable_flushControlled, | |||
| unstable_isNewReconciler, | |||
There was a problem hiding this comment.
I tried a quick code search, but I didn't see any place reading this value internally. Not sure I missed something if anyone remembers the reason this was exposed.
There was a problem hiding this comment.
It was used in www at one point to detect which version of the reconciler was enabled at runtime. Shouldn't need it anymore.
| // NOTE: I did not put much thought into how to configure this. | ||
| __VARIANT__: bundle.enableNewReconciler === true, | ||
| // NOTE: Should only be used in test files. | ||
| __VARIANT__: true, |
There was a problem hiding this comment.
I think this is just for the released bundles, so it shouldn't actually be ever found in files that end up in the bundle? I only found uses of the __VARIANT__ variable from tests and the feature flag files.
There was a problem hiding this comment.
Yeah I think you can remove it
There was a problem hiding this comment.
Actually I retract my objection. If/when I want to use this again in the future, I'll implement it a different way that forks at the entire package level, rather than per-file. I think originally we did the per-file thing because it started as only forking a handful of files but it evolved into covering essentially the whole thing.
|
Do you mind renaming the files in a PR all by itself so that the history is preserved? And so existing PRs can be rebased cleanly. Something like:
I messed this up when I introduced the forked files originally because, although I did it in multiple commits, I only had a single PR and they got squashed when I merged it 😆 |
|
Step one is in #25774 now. |
We've heard from multiple contributors that the Reconciler forking mechanism was confusing and/or annoying to deal with. Since it's currently unused and there's no immediate plans to start using it again, this removes the forking.