Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Remove module pattern function component support" #28670

Merged
merged 1 commit into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -225,7 +225,7 @@ export function getInternalReactConstants(version: string): {
HostSingleton: 27, // Same as above
HostText: 6,
IncompleteClassComponent: 17,
IndeterminateComponent: 2, // removed in 19.0.0
IndeterminateComponent: 2,
LazyComponent: 16,
LegacyHiddenComponent: 23,
MemoComponent: 14,
Expand Down
68 changes: 68 additions & 0 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ let act;
let React;
let ReactDOM;
let ReactDOMClient;
let PropTypes;
let findDOMNode;

const clone = function (o) {
Expand Down Expand Up @@ -98,6 +99,7 @@ describe('ReactComponentLifeCycle', () => {
findDOMNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode;
ReactDOMClient = require('react-dom/client');
PropTypes = require('prop-types');
});

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

if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) {
// @gate !disableLegacyContext
it('calls effects on module-pattern component', async () => {
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 root = ReactDOMClient.createRoot(document.createElement('div'));
await expect(async () => {
await act(() => {
root.render(<Parent ref={c => c && log.push('ref')} />);
});
}).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.",
);
await act(() => {
root.render(<Parent ref={c => c && log.push('ref')} />);
});

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

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

it('should warn if getDerivedStateFromProps returns undefined', async () => {
class MyComponent extends React.Component {
state = {};
Expand Down
74 changes: 55 additions & 19 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,27 +211,63 @@ describe('ReactCompositeComponent', () => {
});
});

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

const el = document.createElement('div');
const root = ReactDOMClient.createRoot(el);
await expect(async () => {
await act(() => {
root.render(<Child test="test" />);
});
}).rejects.toThrow(
'Objects are not valid as a React child (found: object with keys {render}).',
);
const el = document.createElement('div');
const root = ReactDOMClient.createRoot(el);
await expect(async () => {
await expect(async () => {
await act(() => {
root.render(<Child test="test" />);
});
}).rejects.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('');
});
expect(el.textContent).toBe('');
});
} else {
it('should support module pattern components', () => {
function Child({test}) {
return {
render() {
return <div>{test}</div>;
},
};
}

const el = document.createElement('div');
const root = ReactDOMClient.createRoot(el);
expect(() => {
ReactDOM.flushSync(() => {
root.render(<Child test="test" />);
});
}).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('test');
});
}

it('should use default values for undefined props', async () => {
class Component extends React.Component {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,72 @@ describe('ReactCompositeComponent-state', () => {
]);
});

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

const el = document.createElement('div');
const root = ReactDOMClient.createRoot(el);
expect(() => {
ReactDOM.flushSync(() => {
root.render(<Child />);
});
}).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', async () => {
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');
const root = ReactDOMClient.createRoot(el);
await act(() => {
root.render(<Child incrementBy={0} />);
});

expect(el.textContent).toBe('count:1');
await act(() => {
root.render(<Child incrementBy={2} />);
});
expect(el.textContent).toBe('count:3');

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

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

itThrowsWhenRendering(
'factory components',
async render => {
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 => {
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})',
);
checkFooDiv(await render(<FactoryComponent />, 1));
});
}
});

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

// @gate !disableModulePatternComponents
it('renders an error state if module-style context provider throws in componentWillMount', async () => {
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');
const root = ReactDOMClient.createRoot(container);

await expect(async () => {
await act(() => {
root.render(
<ErrorBoundary>
<BrokenComponentWillMountWithContext />
</ErrorBoundary>,
);
});
}).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.",
...gate(flags =>
flags.disableLegacyContext
? [
'Warning: BrokenComponentWillMountWithContext uses the legacy childContextTypes API which was removed in React 19. Use React.createContext() instead.',
'Warning: BrokenComponentWillMountWithContext uses the legacy childContextTypes API which was removed in React 19. Use React.createContext() instead.',
]
: [],
),
]);

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

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