From bfd6b6ff96768288158990230cb25afcf3bc07c1 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Thu, 11 Jul 2024 15:43:43 -0400 Subject: [PATCH] Feature flag to disable legacy context for function components While the goal is to remove legacy context completely, I think we can already land the removal of legacy context for function components. I didn't even know this feature existed until reading the code recently. The win is just a couple of property lookups on function renders, but it trims down the API already as the full removal will likely still take a bit more time. www: Starting with enabled test renderer and a feature flag for production rollout. RN: Not enabled, will follow up on this. --- ...tDOMServerIntegrationLegacyContext-test.js | 24 +++++++++++++++++++ .../__tests__/ReactFunctionComponent-test.js | 2 +- .../src/ReactFiberBeginWork.js | 3 ++- .../src/__tests__/ReactIncremental-test.js | 6 ++--- ...tIncrementalErrorHandling-test.internal.js | 2 +- .../src/__tests__/ReactUse-test.js | 2 +- packages/react-server/src/ReactFizzServer.js | 3 ++- packages/shared/ReactFeatureFlags.js | 8 ++++++- .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + ...actFeatureFlags.test-renderer.native-fb.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 15 files changed, 48 insertions(+), 9 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js index 130bd2310e24a..08551150a7a87 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js @@ -86,6 +86,9 @@ describe('ReactDOMServerIntegration', () => { }); itRenders('stateless child with context', async render => { + if (gate(flags => flags.disableLegacyContextForFunctionComponents)) { + return; + } function FunctionChildWithContext(props, context) { return
{context.text}
; } @@ -118,6 +121,9 @@ describe('ReactDOMServerIntegration', () => { }); itRenders('stateless child without context', async render => { + if (gate(flags => flags.disableLegacyContextForFunctionComponents)) { + return; + } function FunctionChildWithoutContext(props, context) { // this should render blank; context isn't passed to this component. return
{context.text}
; @@ -151,6 +157,9 @@ describe('ReactDOMServerIntegration', () => { }); itRenders('stateless child with wrong context', async render => { + if (gate(flags => flags.disableLegacyContextForFunctionComponents)) { + return; + } function FunctionChildWithWrongContext(props, context) { // this should render blank; context.text isn't passed to this component. return
{context.text}
; @@ -169,6 +178,9 @@ describe('ReactDOMServerIntegration', () => { }); itRenders('with context passed through to a grandchild', async render => { + if (gate(flags => flags.disableLegacyContextForFunctionComponents)) { + return; + } function Grandchild(props, context) { return
{context.text}
; } @@ -186,6 +198,9 @@ describe('ReactDOMServerIntegration', () => { }); itRenders('a child context overriding a parent context', async render => { + if (gate(flags => flags.disableLegacyContextForFunctionComponents)) { + return; + } const Grandchild = (props, context) => { return
{context.text}
; }; @@ -203,6 +218,9 @@ describe('ReactDOMServerIntegration', () => { }); itRenders('a child context merged with a parent context', async render => { + if (gate(flags => flags.disableLegacyContextForFunctionComponents)) { + return; + } class Parent extends React.Component { getChildContext() { return {text1: 'purple'}; @@ -244,6 +262,9 @@ describe('ReactDOMServerIntegration', () => { itRenders( 'with a call to componentWillMount before getChildContext', async render => { + if (gate(flags => flags.disableLegacyContextForFunctionComponents)) { + return; + } class WillMountContext extends React.Component { getChildContext() { return {text: this.state.text}; @@ -270,6 +291,9 @@ describe('ReactDOMServerIntegration', () => { itRenders( 'if getChildContext exists but childContextTypes is missing with a warning', async render => { + if (gate(flags => flags.disableLegacyContextForFunctionComponents)) { + return; + } function HopefulChild(props, context) { return context.foo || 'nope'; } diff --git a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js index d2b1aaa4346d0..6a70f22db917c 100644 --- a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js @@ -451,7 +451,7 @@ describe('ReactFunctionComponent', () => { ]); }); - // @gate !disableLegacyContext + // @gate !disableLegacyContext && !disableLegacyContextForFunctionComponents it('should receive context', async () => { class Parent extends React.Component { static childContextTypes = { diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index d53ee83777cb0..98a89e018da97 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -95,6 +95,7 @@ import { import { debugRenderPhaseSideEffectsForStrictMode, disableLegacyContext, + disableLegacyContextForFunctionComponents, enableProfilerCommitHooks, enableProfilerTimer, enableScopeAPI, @@ -1158,7 +1159,7 @@ function updateFunctionComponent( } let context; - if (!disableLegacyContext) { + if (!disableLegacyContext && !disableLegacyContextForFunctionComponents) { const unmaskedContext = getUnmaskedContext(workInProgress, Component, true); context = getMaskedContext(workInProgress, unmaskedContext); } diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.js index 399e4c01b98cb..50ed1ba5ee9b6 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncremental-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncremental-test.js @@ -1701,7 +1701,7 @@ describe('ReactIncremental', () => { expect(instance.state.n).toEqual(3); }); - // @gate !disableLegacyContext + // @gate !disableLegacyContext && !disableLegacyContextForFunctionComponents it('merges and masks context', async () => { class Intl extends React.Component { static childContextTypes = { @@ -1954,7 +1954,7 @@ describe('ReactIncremental', () => { ]); }); - // @gate !disableLegacyContext + // @gate !disableLegacyContext && !disableLegacyContextForFunctionComponents it('reads context when setState is below the provider', async () => { let statefulInst; @@ -2046,7 +2046,7 @@ describe('ReactIncremental', () => { assertLog([]); }); - // @gate !disableLegacyContext + // @gate !disableLegacyContext && !disableLegacyContextForFunctionComponents it('reads context when setState is above the provider', async () => { let statefulInst; diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 91658c562c948..2f0f23dca895b 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1158,7 +1158,7 @@ describe('ReactIncrementalErrorHandling', () => { // because it's used for new context, suspense, and many other features. // It has to be tested independently for each feature anyway. So although it // doesn't look like it, this test is specific to legacy context. - // @gate !disableLegacyContext + // @gate !disableLegacyContext && !disableLegacyContextForFunctionComponents it('unwinds the context stack correctly on error', async () => { class Provider extends React.Component { static childContextTypes = {message: PropTypes.string}; diff --git a/packages/react-reconciler/src/__tests__/ReactUse-test.js b/packages/react-reconciler/src/__tests__/ReactUse-test.js index bdc13e0da939d..f648f76aefe53 100644 --- a/packages/react-reconciler/src/__tests__/ReactUse-test.js +++ b/packages/react-reconciler/src/__tests__/ReactUse-test.js @@ -1562,7 +1562,7 @@ describe('ReactUse', () => { expect(root).toMatchRenderedOutput('Async!'); }); - // @gate !disableLegacyContext + // @gate !disableLegacyContext && !disableLegacyContextForFunctionComponents it('unwrap uncached promises in component that accesses legacy context', async () => { class ContextProvider extends React.Component { static childContextTypes = { diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 8fb89f717226f..e068db78dcf8a 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -152,6 +152,7 @@ import { import ReactSharedInternals from 'shared/ReactSharedInternals'; import { disableLegacyContext, + disableLegacyContextForFunctionComponents, enableScopeAPI, enableSuspenseAvoidThisFallbackFizz, enableCache, @@ -1654,7 +1655,7 @@ function renderFunctionComponent( props: any, ): void { let legacyContext; - if (!disableLegacyContext) { + if (!disableLegacyContext && !disableLegacyContextForFunctionComponents) { legacyContext = getMaskedContext(Component, task.legacyContext); } if (__DEV__) { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index a3280444766c7..3c2089cb19c13 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -155,8 +155,14 @@ export const transitionLaneExpirationMs = 5000; // Renames the internal symbol for elements since they have changed signature/constructor export const renameElementSymbol = true; -// Removes legacy style context +/** + * Removes legacy style context defined using static `contextTypes` and consumed with static `childContextTypes`. + */ export const disableLegacyContext = true; +/** + * Removes legacy style context just from function components. + */ +export const disableLegacyContextForFunctionComponents = true; // Not ready to break experimental yet. // Modern behaviour aligns more with what components diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 0939b459a9b40..d2a67032cedff 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -37,6 +37,7 @@ export const disableCommentsAsDOMContainers = true; export const disableIEWorkarounds = true; export const disableInputAttributeSyncing = false; export const disableLegacyContext = false; +export const disableLegacyContextForFunctionComponents = false; export const disableLegacyMode = false; export const disableSchedulerTimeoutInWorkLoop = false; export const disableStringRefs = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 031f20247eda2..15caf9713bf75 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -28,6 +28,7 @@ export const disableDefaultPropsExceptForClasses = true; export const disableIEWorkarounds = true; export const disableInputAttributeSyncing = false; export const disableLegacyContext = true; +export const disableLegacyContextForFunctionComponents = true; export const disableLegacyMode = false; export const disableSchedulerTimeoutInWorkLoop = false; export const disableStringRefs = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 172335d7cc735..5b01abdb496bf 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -92,6 +92,7 @@ export const disableStringRefs = true; export const enableFastJSX = true; export const disableLegacyMode = true; export const disableLegacyContext = true; +export const disableLegacyContextForFunctionComponents = true; export const enableRenderableContext = true; export const enableReactTestRendererWarning = true; export const disableDefaultPropsExceptForClasses = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index cea3b6ada831d..d5429617569ca 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -20,6 +20,7 @@ export const disableDefaultPropsExceptForClasses = false; export const disableIEWorkarounds = true; export const disableInputAttributeSyncing = false; export const disableLegacyContext = false; +export const disableLegacyContextForFunctionComponents = false; export const disableLegacyMode = false; export const disableSchedulerTimeoutInWorkLoop = false; export const disableStringRefs = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index bdc43fd844294..026c2be4f8c67 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -32,6 +32,7 @@ export const enableScopeAPI = true; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = true; export const disableLegacyContext = false; +export const disableLegacyContextForFunctionComponents = false; export const enableTrustedTypesIntegration = false; export const disableTextareaChildren = false; export const enableSuspenseAvoidThisFallback = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 9c6e40b373827..34a8ff82fa32a 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -15,6 +15,7 @@ export const alwaysThrottleRetries = true; export const disableDefaultPropsExceptForClasses = __VARIANT__; +export const disableLegacyContextForFunctionComponents = __VARIANT__; export const disableLegacyMode = __VARIANT__; export const disableSchedulerTimeoutInWorkLoop = __VARIANT__; export const enableAddPropertiesFastPath = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 00105c2ff6263..3671f40ad8d31 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -17,6 +17,7 @@ const dynamicFeatureFlags: DynamicFeatureFlags = require('ReactFeatureFlags'); export const { alwaysThrottleRetries, disableDefaultPropsExceptForClasses, + disableLegacyContextForFunctionComponents, disableSchedulerTimeoutInWorkLoop, enableAddPropertiesFastPath, enableDebugTracing,