Skip to content

Commit

Permalink
Remove module pattern function component support (#27742)
Browse files Browse the repository at this point in the history 
The module pattern

```
function MyComponent() {
  return {
    render() {
      return this.state.foo
    }
  }
}
```

has been deprecated for approximately 5 years now. This PR removes
support for this pattern. It also simplifies a number of code paths in
particular related to the concept of `IndeterminateComponent` types.
  • Loading branch information
@gnoff
gnoff authored Mar 28, 2024
1 parent 63651c4 commit cc56bed
Show file tree
Hide file tree
Showing 31 changed files with 124 additions and 923 deletions.
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backend/renderer.js
View file
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,
IndeterminateComponent: 2, // removed in 19.0.0
LazyComponent: 16,
LegacyHiddenComponent: 23,
MemoComponent: 14,
Expand Down
68 changes: 0 additions & 68 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ let act;
let React;
let ReactDOM;
let ReactDOMClient;
let PropTypes;
let findDOMNode;

const clone = function (o) {
Expand Down Expand Up @@ -99,7 +98,6 @@ 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 @@ -1114,72 +1112,6 @@ 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: 19 additions & 55 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,63 +211,27 @@ describe('ReactCompositeComponent', () => {
});
});

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 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('');
});
} else {
it('should support module pattern components', () => {
function Child({test}) {
return {
render() {
return <div>{test}</div>;
},
};
}
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);
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.",
);
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}).',
);

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

it('should use default values for undefined props', async () => {
class Component extends React.Component {
Expand Down
66 changes: 0 additions & 66 deletions packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,72 +527,6 @@ 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
27 changes: 7 additions & 20 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -627,33 +627,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
50 changes: 0 additions & 50 deletions packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -879,56 +879,6 @@ 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

0 comments on commit cc56bed

Please sign in to comment.