Remove disableClientCache feature flag#36033
Open
ShehabSherif0 wants to merge 1 commit intofacebook:mainfrom
Open
Remove disableClientCache feature flag#36033ShehabSherif0 wants to merge 1 commit intofacebook:mainfrom
ShehabSherif0 wants to merge 1 commit intofacebook:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Removes the disableClientCache feature flag (previously hardcoded true everywhere) and simplifies client-side React.cache to always use the no-op implementation, eliminating unreachable flag-guarded code paths.
Changes:
- Deleted
disableClientCachefromReactFeatureFlags.jsand all relevant fork flag files. - Simplified
packages/react/src/ReactCacheClient.jsto unconditionally export the no-opcache/cacheSignal. - Removed the www Jest test override and the
@gate disableClientCacheannotation from a markup client test.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/jest/setupTests.www.js | Removes the __VARIANT__ override for a now-deleted flag in www Jest setup. |
| packages/shared/ReactFeatureFlags.js | Removes the disableClientCache flag definition from shared feature flags. |
| packages/shared/forks/ReactFeatureFlags.www.js | Removes the flag from the www fork. |
| packages/shared/forks/ReactFeatureFlags.test-renderer.www.js | Removes the flag from the test-renderer www fork. |
| packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js | Removes the flag from the test-renderer native-fb fork. |
| packages/shared/forks/ReactFeatureFlags.test-renderer.js | Removes the flag from the test-renderer fork. |
| packages/shared/forks/ReactFeatureFlags.native-oss.js | Removes the flag from the native-oss fork. |
| packages/shared/forks/ReactFeatureFlags.native-fb.js | Removes the flag from the native-fb fork. |
| packages/react/src/ReactCacheClient.js | Makes cache and cacheSignal always export no-ops (no conditional import/branch). |
| packages/react-markup/src/tests/ReactMarkupClient-test.js | Removes @gate disableClientCache since the flag no longer exists. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
c797118 to
99badc9
Compare
99badc9 to
35814df
Compare
|
Nice cleanup! Removing dead code paths simplifies the codebase. The changes look correct - the flag was \ rue\ everywhere, so the conditional branch was never executed. |
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
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
disableClientCachefeature flag is set totruein all environments (main flags file, www, native-fb, native-oss, test-renderer, test-renderer.www, test-renderer.native-fb). This PR removes the flag and the dead code path it guarded.When
disableClientCachewastrue,ReactCacheClient.jsexportednoopCacheandnoopCacheSignal(no-ops that pass through or return null). With the flag hardcoded totrueeverywhere, the conditional branch that imported the real cache implementation fromReactCacheImpl.jswas unreachable dead code.This PR:
disableClientCacheflag fromReactFeatureFlags.jsand all 6 fork filesReactCacheClient.jsto unconditionally export the no-op implementations__VARIANT__test override insetupTests.www.js@gate disableClientCacheannotation from a test inReactMarkupClient-test.jsNote:
ReactCacheServer.jsis unaffected -- it imports directly fromReactCacheImpl.jswithout any flag check.How did you test this change?
yarn linc(lint on changed files): passedyarn test(dev mode, experimental): all cache and markup tests passed (23/23)yarn test --prod(production mode, experimental): all tests passed (23/23)yarn test -r www-modern(www variant): all tests passed (6/6)yarn flow(Flow type checks): 0 errorsdisableClientCacheacross the codebase