Skip to content

Commit

Permalink
React has deprecated module pattern Function Components for many year…
Browse files Browse the repository at this point in the history
…s at this point. Supporting this pattern required React to have a concept of an indeterminate component so that when a component first renders it can turn into either a ClassComponent or a FunctionComponent depending on what it returns. While this feature was deprecated and put behind a flag it is still in stable. This change remvoes the flag, removes the warnings, and removes the concept of IndeterminateComponent from the React codebase.
  • Loading branch information
gnoff committed Nov 29, 2023
1 parent 6c7b41d commit 2e5b24b
Show file tree
Hide file tree
Showing 33 changed files with 119 additions and 855 deletions.
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ export function getInternalReactConstants(version: string): {
HostSingleton: 27, // Same as above
HostText: 6,
IncompleteClassComponent: 17,
IndeterminateComponent: 2,
IndeterminateComponent: 2, // removed in 19.0.0
LazyComponent: 16,
LegacyHiddenComponent: 23,
MemoComponent: 14,
Expand Down
63 changes: 0 additions & 63 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
let React;
let ReactDOM;
let ReactTestUtils;
let PropTypes;

const clone = function (o) {
return JSON.parse(JSON.stringify(o));
Expand Down Expand Up @@ -91,7 +90,6 @@ describe('ReactComponentLifeCycle', () => {
React = require('react');
ReactDOM = require('react-dom');
ReactTestUtils = require('react-dom/test-utils');
PropTypes = require('prop-types');
});

it('should not reuse an instance when it has been unmounted', () => {
Expand Down Expand Up @@ -983,67 +981,6 @@ describe('ReactComponentLifeCycle', () => {
});
});

if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) {
it('calls effects on module-pattern component', function () {
const log = [];

function Parent() {
return {
render() {
expect(typeof this.props).toBe('object');
log.push('render');
return <Child />;
},
UNSAFE_componentWillMount() {
log.push('will mount');
},
componentDidMount() {
log.push('did mount');
},
componentDidUpdate() {
log.push('did update');
},
getChildContext() {
return {x: 2};
},
};
}
Parent.childContextTypes = {
x: PropTypes.number,
};
function Child(props, context) {
expect(context.x).toBe(2);
return <div />;
}
Child.contextTypes = {
x: PropTypes.number,
};

const div = document.createElement('div');
expect(() =>
ReactDOM.render(<Parent ref={c => c && log.push('ref')} />, div),
).toErrorDev(
'Warning: The <Parent /> component appears to be a function component that returns a class instance. ' +
'Change Parent to a class that extends React.Component instead. ' +
"If you can't use a class try assigning the prototype on the function as a workaround. " +
'`Parent.prototype = React.Component.prototype`. ' +
"Don't use an arrow function since it cannot be called with `new` by React.",
);
ReactDOM.render(<Parent ref={c => c && log.push('ref')} />, div);

expect(log).toEqual([
'will mount',
'render',
'did mount',
'ref',

'render',
'did update',
'ref',
]);
});
}

it('should warn if getDerivedStateFromProps returns undefined', () => {
class MyComponent extends React.Component {
state = {};
Expand Down
59 changes: 14 additions & 45 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,53 +111,22 @@ describe('ReactCompositeComponent', () => {
};
});

if (require('shared/ReactFeatureFlags').disableModulePatternComponents) {
it('should not support module pattern components', () => {
function Child({test}) {
return {
render() {
return <div>{test}</div>;
},
};
}

const el = document.createElement('div');
expect(() => {
expect(() => ReactDOM.render(<Child test="test" />, el)).toThrow(
'Objects are not valid as a React child (found: object with keys {render}).',
);
}).toErrorDev(
'Warning: The <Child /> component appears to be a function component that returns a class instance. ' +
'Change Child to a class that extends React.Component instead. ' +
"If you can't use a class try assigning the prototype on the function as a workaround. " +
'`Child.prototype = React.Component.prototype`. ' +
"Don't use an arrow function since it cannot be called with `new` by React.",
);

expect(el.textContent).toBe('');
});
} else {
it('should support module pattern components', () => {
function Child({test}) {
return {
render() {
return <div>{test}</div>;
},
};
}
it('should not support module pattern components', () => {
function Child({test}) {
return {
render() {
return <div>{test}</div>;
},
};
}

const el = document.createElement('div');
expect(() => ReactDOM.render(<Child test="test" />, el)).toErrorDev(
'Warning: The <Child /> component appears to be a function component that returns a class instance. ' +
'Change Child to a class that extends React.Component instead. ' +
"If you can't use a class try assigning the prototype on the function as a workaround. " +
'`Child.prototype = React.Component.prototype`. ' +
"Don't use an arrow function since it cannot be called with `new` by React.",
);
const el = document.createElement('div');
expect(() => ReactDOM.render(<Child test="test" />, el)).toThrow(
'Objects are not valid as a React child (found: object with keys {render}).',
);

expect(el.textContent).toBe('test');
});
}
expect(el.textContent).toBe('');
});

it('should support rendering to different child types over time', () => {
const instance = ReactTestUtils.renderIntoDocument(<MorphingComponent />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,60 +457,6 @@ describe('ReactCompositeComponent-state', () => {
]);
});

if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) {
it('should support stateful module pattern components', () => {
function Child() {
return {
state: {
count: 123,
},
render() {
return <div>{`count:${this.state.count}`}</div>;
},
};
}

const el = document.createElement('div');
expect(() => ReactDOM.render(<Child />, el)).toErrorDev(
'Warning: The <Child /> component appears to be a function component that returns a class instance. ' +
'Change Child to a class that extends React.Component instead. ' +
"If you can't use a class try assigning the prototype on the function as a workaround. " +
'`Child.prototype = React.Component.prototype`. ' +
"Don't use an arrow function since it cannot be called with `new` by React.",
);

expect(el.textContent).toBe('count:123');
});

it('should support getDerivedStateFromProps for module pattern components', () => {
function Child() {
return {
state: {
count: 1,
},
render() {
return <div>{`count:${this.state.count}`}</div>;
},
};
}
Child.getDerivedStateFromProps = (props, prevState) => {
return {
count: prevState.count + props.incrementBy,
};
};

const el = document.createElement('div');
ReactDOM.render(<Child incrementBy={0} />, el);
expect(el.textContent).toBe('count:1');

ReactDOM.render(<Child incrementBy={2} />, el);
expect(el.textContent).toBe('count:3');

ReactDOM.render(<Child incrementBy={1} />, el);
expect(el.textContent).toBe('count:4');
});
}

it('should support setState in componentWillUnmount', () => {
let subscription;
class A extends React.Component {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -622,33 +622,20 @@ describe('ReactDOMServerIntegration', () => {
checkFooDiv(await render(<ClassComponent />));
});

if (require('shared/ReactFeatureFlags').disableModulePatternComponents) {
itThrowsWhenRendering(
'factory components',
async render => {
const FactoryComponent = () => {
return {
render: function () {
return <div>foo</div>;
},
};
};
await render(<FactoryComponent />, 1);
},
'Objects are not valid as a React child (found: object with keys {render})',
);
} else {
itRenders('factory components', async render => {
itThrowsWhenRendering(
'factory components',
async render => {
const FactoryComponent = () => {
return {
render: function () {
return <div>foo</div>;
},
};
};
checkFooDiv(await render(<FactoryComponent />, 1));
});
}
await render(<FactoryComponent />, 1);
},
'Objects are not valid as a React child (found: object with keys {render})',
);
});

describe('component hierarchies', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -799,46 +799,6 @@ describe('ReactErrorBoundaries', () => {
expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
});

// @gate !disableModulePatternComponents
// @gate !disableLegacyContext
it('renders an error state if module-style context provider throws in componentWillMount', () => {
function BrokenComponentWillMountWithContext() {
return {
getChildContext() {
return {foo: 42};
},
render() {
return <div>{this.props.children}</div>;
},
UNSAFE_componentWillMount() {
throw new Error('Hello');
},
};
}
BrokenComponentWillMountWithContext.childContextTypes = {
foo: PropTypes.number,
};

const container = document.createElement('div');
expect(() =>
ReactDOM.render(
<ErrorBoundary>
<BrokenComponentWillMountWithContext />
</ErrorBoundary>,
container,
),
).toErrorDev(
'Warning: The <BrokenComponentWillMountWithContext /> component appears to be a function component that ' +
'returns a class instance. ' +
'Change BrokenComponentWillMountWithContext to a class that extends React.Component instead. ' +
"If you can't use a class try assigning the prototype on the function as a workaround. " +
'`BrokenComponentWillMountWithContext.prototype = React.Component.prototype`. ' +
"Don't use an arrow function since it cannot be called with `new` by React.",
);

expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
});

it('mounts the error message if mounting fails', () => {
function renderError(error) {
return <ErrorMessage message={error.message} />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -825,45 +825,6 @@ describe('ReactLegacyErrorBoundaries', () => {
expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
});

if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) {
it('renders an error state if module-style context provider throws in componentWillMount', () => {
function BrokenComponentWillMountWithContext() {
return {
getChildContext() {
return {foo: 42};
},
render() {
return <div>{this.props.children}</div>;
},
UNSAFE_componentWillMount() {
throw new Error('Hello');
},
};
}
BrokenComponentWillMountWithContext.childContextTypes = {
foo: PropTypes.number,
};

const container = document.createElement('div');
expect(() =>
ReactDOM.render(
<ErrorBoundary>
<BrokenComponentWillMountWithContext />
</ErrorBoundary>,
container,
),
).toErrorDev(
'Warning: The <BrokenComponentWillMountWithContext /> component appears to be a function component that ' +
'returns a class instance. ' +
'Change BrokenComponentWillMountWithContext to a class that extends React.Component instead. ' +
"If you can't use a class try assigning the prototype on the function as a workaround. " +
'`BrokenComponentWillMountWithContext.prototype = React.Component.prototype`. ' +
"Don't use an arrow function since it cannot be called with `new` by React.",
);
expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
});
}

it('mounts the error message if mounting fails', () => {
function renderError(error) {
return <ErrorMessage message={error.message} />;
Expand Down
Loading

0 comments on commit 2e5b24b

Please sign in to comment.