Skip to content

Commit

Permalink
Feature flag to disable legacy context for function components
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kassens committed Jul 11, 2024
1 parent d09484c commit bfd6b6f
Show file tree
Hide file tree
Showing 15 changed files with 48 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ describe('ReactDOMServerIntegration', () => {
});

itRenders('stateless child with context', async render => {
if (gate(flags => flags.disableLegacyContextForFunctionComponents)) {
return;
}
function FunctionChildWithContext(props, context) {
return <div>{context.text}</div>;
}
Expand Down Expand Up @@ -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 <div>{context.text}</div>;
Expand Down Expand Up @@ -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 <div id="statelessWrongChild">{context.text}</div>;
Expand All @@ -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 <div>{context.text}</div>;
}
Expand All @@ -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 <div>{context.text}</div>;
};
Expand All @@ -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'};
Expand Down Expand Up @@ -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};
Expand All @@ -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';
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ describe('ReactFunctionComponent', () => {
]);
});

// @gate !disableLegacyContext
// @gate !disableLegacyContext && !disableLegacyContextForFunctionComponents
it('should receive context', async () => {
class Parent extends React.Component {
static childContextTypes = {
Expand Down
3 changes: 2 additions & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ import {
import {
debugRenderPhaseSideEffectsForStrictMode,
disableLegacyContext,
disableLegacyContextForFunctionComponents,
enableProfilerCommitHooks,
enableProfilerTimer,
enableScopeAPI,
Expand Down Expand Up @@ -1158,7 +1159,7 @@ function updateFunctionComponent(
}

let context;
if (!disableLegacyContext) {
if (!disableLegacyContext && !disableLegacyContextForFunctionComponents) {
const unmaskedContext = getUnmaskedContext(workInProgress, Component, true);
context = getMaskedContext(workInProgress, unmaskedContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -1954,7 +1954,7 @@ describe('ReactIncremental', () => {
]);
});

// @gate !disableLegacyContext
// @gate !disableLegacyContext && !disableLegacyContextForFunctionComponents
it('reads context when setState is below the provider', async () => {
let statefulInst;

Expand Down Expand Up @@ -2046,7 +2046,7 @@ describe('ReactIncremental', () => {
assertLog([]);
});

// @gate !disableLegacyContext
// @gate !disableLegacyContext && !disableLegacyContextForFunctionComponents
it('reads context when setState is above the provider', async () => {
let statefulInst;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/__tests__/ReactUse-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
3 changes: 2 additions & 1 deletion packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ import {
import ReactSharedInternals from 'shared/ReactSharedInternals';
import {
disableLegacyContext,
disableLegacyContextForFunctionComponents,
enableScopeAPI,
enableSuspenseAvoidThisFallbackFizz,
enableCache,
Expand Down Expand Up @@ -1654,7 +1655,7 @@ function renderFunctionComponent(
props: any,
): void {
let legacyContext;
if (!disableLegacyContext) {
if (!disableLegacyContext && !disableLegacyContextForFunctionComponents) {
legacyContext = getMaskedContext(Component, task.legacyContext);
}
if (__DEV__) {
Expand Down
8 changes: 7 additions & 1 deletion packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <StrictMode /> behaviour aligns more with what components
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -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__;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const dynamicFeatureFlags: DynamicFeatureFlags = require('ReactFeatureFlags');
export const {
alwaysThrottleRetries,
disableDefaultPropsExceptForClasses,
disableLegacyContextForFunctionComponents,
disableSchedulerTimeoutInWorkLoop,
enableAddPropertiesFastPath,
enableDebugTracing,
Expand Down

0 comments on commit bfd6b6f

Please sign in to comment.