Clean up enableEagerAlternateStateNodeCleanup#57358
Closed
javache wants to merge 2 commits into
Closed
Conversation
Summary: ## Summary The `runtime_commit_artifacts` workflow has been failing in the "move relevant files for react in fbsource into compiled-rn" step with: ``` mv: cannot stat 'build/facebook-react-native/VERSION_NATIVE_FB': No such file or directory ``` `VERSION_NATIVE_FB` was only written in `processExperimental`, *inside* `build/facebook-react-native/`. That directory is not part of the allowlist in the cleanup loop at the end of `processExperimental`, so the experimental workers wrote the version file and then immediately deleted it. This was masked until react#36859. The cleanup loop previously used `rm -rm` (an invalid flag), so `rm` errored out and deleted nothing — the loop was effectively a no-op. Once it was changed to `fs.rmSync`, the deletion actually took effect. After CI merges the per-worker artifacts, `facebook-react-native/*/cjs` still survives (the **stable** workers produce it and `processStable` has no cleanup loop), but `VERSION_NATIVE_FB` is gone because stable never wrote it. ## Fix The `build/facebook-react-native` artifacts are identical across release channels: - RN packages resolve to the generic `.fb.js` entry fork (no `.modern.fb.js` / `.classic.fb.js` split). - The `native-fb` feature-flag forks don't reference `__EXPERIMENTAL__`. - The version string is identical (`<ReactVersion>-native-fb-<sha>-<date>`). So this writes `VERSION_NATIVE_FB` from `processStable`, where `facebook-react-native` is preserved, and drops the now-dead write from `processExperimental`. ## Test Plan Verified locally by building the `react` package through each code path: - **Stable channel** (`yarn build --r=stable react/index`): `build/facebook-react-native/VERSION_NATIVE_FB` is written with the correct `19.3.0-native-fb-<sha>-<date>` version string. ✅ - **Experimental channel** (`yarn build --r=experimental react/index`): `build/facebook-react-native` is deleted by the cleanup loop, leaving only `oss-experimental` and `facebook-www` — confirming the version file cannot come from this channel. ✅ - **Local merged build** (`yarn build react/index`, builds both channels and merges): the final `build/facebook-react-native/` contains `VERSION_NATIVE_FB`, matching what CI produces after merging per-worker artifacts. ✅ DiffTrain build for [68631c0453b08e2c7c96a40910f4c91db1f66d5a](react/react@68631c0) Reviewed By: rickhanlonii, zeyap Differential Revision: D109860005
Summary: The `enableEagerAlternateStateNodeCleanup` flag is no longer read by the renderer, so the feature-flag definition and the two prelude overrides that forced it on become dead code. Drop the flag entry from `ReactNativeInternalFeatureFlags` (incl. its test mirror and the Fantom mock) and remove the `= true` assignments from the Airwave and FB4TV preludes. The dedicated `VirtualView-enableEagerAlternateStateNodeCleanup-itest.js` is folded into `VirtualView-itest.js`'s `memory management` describe so the shadow-node retention regression test continues to run, just no longer keyed on a non-existent flag. Vendored React (`react-devtools-facade`), the append-only mobileconfig stable_id, and the generated `react-devtools-core` bundles are intentionally untouched. Changelog: [Internal] Differential Revision: D110043719
meta-cla
Bot
added
the
CLA Signed
|
@javache has exported this pull request. If you are a Meta employee, you can view the originating Diff in D110043719. |
|
This pull request has been merged in 610192e. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary:
The
enableEagerAlternateStateNodeCleanupflag is no longer read by the renderer, so the feature-flag definition and the two prelude overrides that forced it on become dead code.Changelog:
[Internal]
Differential Revision: D110043719