Skip to content

Commit acd65db

Browse files
sebmarkbagesebmarkbage
authored
Deprecate module pattern (factory) components (#15145)
1 parent 55cc921 commit acd65db

14 files changed

+151
-41
lines changed

‎packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,16 @@ describe('ReactComponentLifeCycle', () => {
977977
};
978978

979979
const div = document.createElement('div');
980-
ReactDOM.render(<Parent ref={c => c && log.push('ref')} />, div);
980+
expect(() =>
981+
ReactDOM.render(<Parent ref={c => c && log.push('ref')} />, div),
982+
).toWarnDev(
983+
'Warning: The <Parent /> component appears to be a function component that returns a class instance. ' +
984+
'Change Parent to a class that extends React.Component instead. ' +
985+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
986+
'`Parent.prototype = React.Component.prototype`. ' +
987+
"Don't use an arrow function since it cannot be called with `new` by React.",
988+
{withoutStack: true},
989+
);
981990
ReactDOM.render(<Parent ref={c => c && log.push('ref')} />, div);
982991

983992
expect(log).toEqual([

‎packages/react-dom/src/__tests__/ReactCompositeComponent-test.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,14 @@ describe('ReactCompositeComponent', () => {
118118
}
119119

120120
const el = document.createElement('div');
121-
ReactDOM.render(<Child test="test" />, el);
121+
expect(() => ReactDOM.render(<Child test="test" />, el)).toWarnDev(
122+
'Warning: The <Child /> component appears to be a function component that returns a class instance. ' +
123+
'Change Child to a class that extends React.Component instead. ' +
124+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
125+
'`Child.prototype = React.Component.prototype`. ' +
126+
"Don't use an arrow function since it cannot be called with `new` by React.",
127+
{withoutStack: true},
128+
);
122129

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

‎packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,14 @@ describe('ReactCompositeComponent-state', () => {
474474
}
475475

476476
const el = document.createElement('div');
477-
ReactDOM.render(<Child />, el);
477+
expect(() => ReactDOM.render(<Child />, el)).toWarnDev(
478+
'Warning: The <Child /> component appears to be a function component that returns a class instance. ' +
479+
'Change Child to a class that extends React.Component instead. ' +
480+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
481+
'`Child.prototype = React.Component.prototype`. ' +
482+
"Don't use an arrow function since it cannot be called with `new` by React.",
483+
{withoutStack: true},
484+
);
478485

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

‎packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@ describe('ReactDOMServerIntegration', () => {
644644
},
645645
};
646646
};
647-
checkFooDiv(await render(<FactoryComponent />));
647+
checkFooDiv(await render(<FactoryComponent />, 1));
648648
});
649649
});
650650

‎packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -768,12 +768,23 @@ describe('ReactErrorBoundaries', () => {
768768
};
769769

770770
const container = document.createElement('div');
771-
ReactDOM.render(
772-
<ErrorBoundary>
773-
<BrokenComponentWillMountWithContext />
774-
</ErrorBoundary>,
775-
container,
771+
expect(() =>
772+
ReactDOM.render(
773+
<ErrorBoundary>
774+
<BrokenComponentWillMountWithContext />
775+
</ErrorBoundary>,
776+
container,
777+
),
778+
).toWarnDev(
779+
'Warning: The <BrokenComponentWillMountWithContext /> component appears to be a function component that ' +
780+
'returns a class instance. ' +
781+
'Change BrokenComponentWillMountWithContext to a class that extends React.Component instead. ' +
782+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
783+
'`BrokenComponentWillMountWithContext.prototype = React.Component.prototype`. ' +
784+
"Don't use an arrow function since it cannot be called with `new` by React.",
785+
{withoutStack: true},
776786
);
787+
777788
expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
778789
});
779790

‎packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -742,11 +742,21 @@ describe('ReactLegacyErrorBoundaries', () => {
742742
};
743743

744744
const container = document.createElement('div');
745-
ReactDOM.render(
746-
<ErrorBoundary>
747-
<BrokenComponentWillMountWithContext />
748-
</ErrorBoundary>,
749-
container,
745+
expect(() =>
746+
ReactDOM.render(
747+
<ErrorBoundary>
748+
<BrokenComponentWillMountWithContext />
749+
</ErrorBoundary>,
750+
container,
751+
),
752+
).toWarnDev(
753+
'Warning: The <BrokenComponentWillMountWithContext /> component appears to be a function component that ' +
754+
'returns a class instance. ' +
755+
'Change BrokenComponentWillMountWithContext to a class that extends React.Component instead. ' +
756+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
757+
'`BrokenComponentWillMountWithContext.prototype = React.Component.prototype`. ' +
758+
"Don't use an arrow function since it cannot be called with `new` by React.",
759+
{withoutStack: true},
750760
);
751761
expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
752762
});

‎packages/react-dom/src/__tests__/refs-test.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,17 @@ describe('factory components', () => {
166166
};
167167
}
168168

169-
const inst = ReactTestUtils.renderIntoDocument(<Comp />);
169+
let inst;
170+
expect(
171+
() => (inst = ReactTestUtils.renderIntoDocument(<Comp />)),
172+
).toWarnDev(
173+
'Warning: The <Comp /> component appears to be a function component that returns a class instance. ' +
174+
'Change Comp to a class that extends React.Component instead. ' +
175+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
176+
'`Comp.prototype = React.Component.prototype`. ' +
177+
"Don't use an arrow function since it cannot be called with `new` by React.",
178+
{withoutStack: true},
179+
);
170180
expect(inst.refs.elemRef.tagName).toBe('DIV');
171181
});
172182
});

‎packages/react-dom/src/server/ReactPartialRenderer.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ let didWarnDefaultTextareaValue = false;
179179
let didWarnInvalidOptionChildren = false;
180180
const didWarnAboutNoopUpdateForComponent = {};
181181
const didWarnAboutBadClass = {};
182+
const didWarnAboutModulePatternComponent = {};
182183
const didWarnAboutDeprecatedWillMount = {};
183184
const didWarnAboutUndefinedDerivedState = {};
184185
const didWarnAboutUninitializedState = {};
@@ -525,6 +526,24 @@ function resolve(
525526
validateRenderResult(child, Component);
526527
return;
527528
}
529+
530+
if (__DEV__) {
531+
const componentName = getComponentName(Component) || 'Unknown';
532+
if (!didWarnAboutModulePatternComponent[componentName]) {
533+
warningWithoutStack(
534+
false,
535+
'The <%s /> component appears to be a function component that returns a class instance. ' +
536+
'Change %s to a class that extends React.Component instead. ' +
537+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
538+
"`%s.prototype = React.Component.prototype`. Don't use an arrow function since it " +
539+
'cannot be called with `new` by React.',
540+
componentName,
541+
componentName,
542+
componentName,
543+
);
544+
didWarnAboutModulePatternComponent[componentName] = true;
545+
}
546+
}
528547
}
529548

530549
inst.props = element.props;

‎packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,13 +144,15 @@ const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
144144
let didReceiveUpdate: boolean = false;
145145

146146
let didWarnAboutBadClass;
147+
let didWarnAboutModulePatternComponent;
147148
let didWarnAboutContextTypeOnFunctionComponent;
148149
let didWarnAboutGetDerivedStateOnFunctionComponent;
149150
let didWarnAboutFunctionRefs;
150151
export let didWarnAboutReassigningProps;
151152

152153
if (__DEV__) {
153154
didWarnAboutBadClass = {};
155+
didWarnAboutModulePatternComponent = {};
154156
didWarnAboutContextTypeOnFunctionComponent = {};
155157
didWarnAboutGetDerivedStateOnFunctionComponent = {};
156158
didWarnAboutFunctionRefs = {};
@@ -1222,6 +1224,24 @@ function mountIndeterminateComponent(
12221224
typeof value.render === 'function' &&
12231225
value.$$typeof === undefined
12241226
) {
1227+
if (__DEV__) {
1228+
const componentName = getComponentName(Component) || 'Unknown';
1229+
if (!didWarnAboutModulePatternComponent[componentName]) {
1230+
warningWithoutStack(
1231+
false,
1232+
'The <%s /> component appears to be a function component that returns a class instance. ' +
1233+
'Change %s to a class that extends React.Component instead. ' +
1234+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
1235+
"`%s.prototype = React.Component.prototype`. Don't use an arrow function since it " +
1236+
'cannot be called with `new` by React.',
1237+
componentName,
1238+
componentName,
1239+
componentName,
1240+
);
1241+
didWarnAboutModulePatternComponent[componentName] = true;
1242+
}
1243+
}
1244+
12251245
// Proceed under the assumption that this is a class instance
12261246
workInProgress.tag = ClassComponent;
12271247

‎packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1327,7 +1327,14 @@ describe('ReactHooks', () => {
13271327
expect(renderCount).toBe(1);
13281328

13291329
renderCount = 0;
1330-
renderer.update(<Factory />);
1330+
expect(() => renderer.update(<Factory />)).toWarnDev(
1331+
'Warning: The <Factory /> component appears to be a function component that returns a class instance. ' +
1332+
'Change Factory to a class that extends React.Component instead. ' +
1333+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
1334+
'`Factory.prototype = React.Component.prototype`. ' +
1335+
"Don't use an arrow function since it cannot be called with `new` by React.",
1336+
{withoutStack: true},
1337+
);
13311338
expect(renderCount).toBe(1);
13321339
renderCount = 0;
13331340
renderer.update(<Factory />);

‎packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,22 @@ describe('ReactHooksWithNoopRenderer', () => {
137137
};
138138
}
139139
ReactNoop.render(<Counter />);
140-
expect(Scheduler).toFlushAndThrow(
141-
'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for' +
142-
' one of the following reasons:\n' +
143-
'1. You might have mismatching versions of React and the renderer (such as React DOM)\n' +
144-
'2. You might be breaking the Rules of Hooks\n' +
145-
'3. You might have more than one copy of React in the same app\n' +
146-
'See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.',
140+
expect(() =>
141+
expect(Scheduler).toFlushAndThrow(
142+
'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen ' +
143+
'for one of the following reasons:\n' +
144+
'1. You might have mismatching versions of React and the renderer (such as React DOM)\n' +
145+
'2. You might be breaking the Rules of Hooks\n' +
146+
'3. You might have more than one copy of React in the same app\n' +
147+
'See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.',
148+
),
149+
).toWarnDev(
150+
'Warning: The <Counter /> component appears to be a function component that returns a class instance. ' +
151+
'Change Counter to a class that extends React.Component instead. ' +
152+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
153+
'`Counter.prototype = React.Component.prototype`. ' +
154+
"Don't use an arrow function since it cannot be called with `new` by React.",
155+
{withoutStack: true},
147156
);
148157

149158
// Confirm that a subsequent hook works properly.

‎packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2026,8 +2026,15 @@ describe('ReactIncremental', () => {
20262026

20272027
ReactNoop.render(<Recurse />);
20282028
expect(() => expect(Scheduler).toFlushWithoutYielding()).toWarnDev(
2029-
'Legacy context API has been detected within a strict-mode tree: \n\n' +
2030-
'Please update the following components: Recurse',
2029+
[
2030+
'Warning: The <Recurse /> component appears to be a function component that returns a class instance. ' +
2031+
'Change Recurse to a class that extends React.Component instead. ' +
2032+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
2033+
'`Recurse.prototype = React.Component.prototype`. ' +
2034+
"Don't use an arrow function since it cannot be called with `new` by React.",
2035+
'Legacy context API has been detected within a strict-mode tree: \n\n' +
2036+
'Please update the following components: Recurse',
2037+
],
20312038
{withoutStack: true},
20322039
);
20332040
expect(ops).toEqual([

‎packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,8 +1596,15 @@ describe('ReactIncrementalErrorHandling', () => {
15961596
expect(() => {
15971597
expect(Scheduler).toFlushAndThrow('Oops!');
15981598
}).toWarnDev(
1599-
'Legacy context API has been detected within a strict-mode tree: \n\n' +
1600-
'Please update the following components: Provider',
1599+
[
1600+
'Warning: The <Provider /> component appears to be a function component that returns a class instance. ' +
1601+
'Change Provider to a class that extends React.Component instead. ' +
1602+
"If you can't use a class try assigning the prototype on the function as a workaround. " +
1603+
'`Provider.prototype = React.Component.prototype`. ' +
1604+
"Don't use an arrow function since it cannot be called with `new` by React.",
1605+
'Legacy context API has been detected within a strict-mode tree: \n\n' +
1606+
'Please update the following components: Provider',
1607+
],
16011608
{withoutStack: true},
16021609
);
16031610
});

‎packages/react/src/__tests__/ReactStrictMode-test.internal.js

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -835,7 +835,6 @@ describe('ReactStrictMode', () => {
835835
<div>
836836
<LegacyContextConsumer />
837837
<FunctionalLegacyContextConsumer />
838-
<FactoryLegacyContextConsumer />
839838
</div>
840839
);
841840
}
@@ -845,14 +844,6 @@ describe('ReactStrictMode', () => {
845844
return null;
846845
}
847846

848-
function FactoryLegacyContextConsumer() {
849-
return {
850-
render() {
851-
return null;
852-
},
853-
};
854-
}
855-
856847
LegacyContextProvider.childContextTypes = {
857848
color: PropTypes.string,
858849
};
@@ -885,10 +876,6 @@ describe('ReactStrictMode', () => {
885876
color: PropTypes.string,
886877
};
887878

888-
FactoryLegacyContextConsumer.contextTypes = {
889-
color: PropTypes.string,
890-
};
891-
892879
let rendered;
893880

894881
expect(() => {
@@ -898,7 +885,7 @@ describe('ReactStrictMode', () => {
898885
'\n in StrictMode (at **)' +
899886
'\n in div (at **)' +
900887
'\n in Root (at **)' +
901-
'\n\nPlease update the following components: FactoryLegacyContextConsumer, ' +
888+
'\n\nPlease update the following components: ' +
902889
'FunctionalLegacyContextConsumer, LegacyContextConsumer, LegacyContextProvider' +
903890
'\n\nLearn more about this warning here:' +
904891
'\nhttps://fb.me/react-strict-mode-warnings',

0 commit comments

Comments
 (0)