From 22cab1cbd6ad52925828643c8c6d0d4fd7890c54 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 17 Mar 2020 22:47:24 +0100 Subject: [PATCH] test(getComponentName): Increase test coverage (#18149) Co-authored-by: Brian Vaughn --- .../__tests__/ReactCompositeComponent-test.js | 6 ++-- ...tDOMServerIntegrationLegacyContext-test.js | 18 ++++++++++ .../ReactDOMServerLifecycles-test.js | 10 +++--- ...eactLegacyContextDisabled-test.internal.js | 28 +++++++++++++++ ...eactHooksWithNoopRenderer-test.internal.js | 36 +++++++++++++++++++ packages/react/src/ReactElement.js | 2 +- 6 files changed, 92 insertions(+), 8 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 49e107f877ef..3767b9aecae5 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -542,7 +542,7 @@ describe('ReactCompositeComponent', () => { }); it('should warn when shouldComponentUpdate() returns undefined', () => { - class Component extends React.Component { + class ClassComponent extends React.Component { state = {bogus: false}; shouldComponentUpdate() { @@ -554,10 +554,10 @@ describe('ReactCompositeComponent', () => { } } - const instance = ReactTestUtils.renderIntoDocument(); + const instance = ReactTestUtils.renderIntoDocument(); expect(() => instance.setState({bogus: true})).toErrorDev( - 'Warning: Component.shouldComponentUpdate(): Returned undefined instead of a ' + + 'Warning: ClassComponent.shouldComponentUpdate(): Returned undefined instead of a ' + 'boolean value. Make sure to return true or false.', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js index 4c78919bd848..5bcfa7b77e72 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationLegacyContext-test.js @@ -291,5 +291,23 @@ describe('ReactDOMServerIntegration', () => { }, 'MyComponent.getChildContext(): key "value2" is not defined in childContextTypes.', ); + + it('warns when childContextTypes is not defined', () => { + class MyComponent extends React.Component { + render() { + return
; + } + getChildContext() { + return {value1: 'foo', value2: 'bar'}; + } + } + + expect(() => { + ReactDOMServer.renderToString(); + }).toErrorDev( + 'Warning: MyComponent.getChildContext(): childContextTypes must be defined in order to use getChildContext().\n' + + ' in MyComponent (at **)', + ); + }); }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js b/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js index 497bf0e3b726..377ced34db98 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js @@ -285,19 +285,21 @@ describe('ReactDOMServerLifecycles', () => { }); it('should warn about deprecated lifecycle hooks', () => { - class Component extends React.Component { + class MyComponent extends React.Component { componentWillMount() {} render() { return null; } } - expect(() => ReactDOMServer.renderToString()).toWarnDev( - 'componentWillMount has been renamed', + expect(() => ReactDOMServer.renderToString()).toWarnDev( + 'componentWillMount has been renamed, and is not recommended for use. See https://fb.me/react-unsafe-component-lifecycles for details.\n\n' + + '* Move code from componentWillMount to componentDidMount (preferred in most cases) or the constructor.\n\n' + + 'Please update the following components: MyComponent', ); // De-duped - ReactDOMServer.renderToString(); + ReactDOMServer.renderToString(); }); describe('react-lifecycles-compat', () => { diff --git a/packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js b/packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js index 28e1b7d4f85e..829373c37eeb 100644 --- a/packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js @@ -11,6 +11,7 @@ let React; let ReactDOM; +let ReactDOMServer; let ReactFeatureFlags; describe('ReactLegacyContextDisabled', () => { @@ -19,6 +20,7 @@ describe('ReactLegacyContextDisabled', () => { React = require('react'); ReactDOM = require('react-dom'); + ReactDOMServer = require('react-dom/server'); ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactFeatureFlags.disableLegacyContext = true; }); @@ -115,6 +117,32 @@ describe('ReactLegacyContextDisabled', () => { expect(container.textContent).toBe('{}undefinedundefined'); expect(lifecycleContextLog).toEqual([{}, {}, {}]); ReactDOM.unmountComponentAtNode(container); + + // test server path. + let text; + expect(() => { + text = ReactDOMServer.renderToString( + + + + + + + , + container, + ); + }).toErrorDev([ + 'LegacyProvider uses the legacy childContextTypes API which is no longer supported. ' + + 'Use React.createContext() instead.', + 'LegacyClsConsumer uses the legacy contextTypes API which is no longer supported. ' + + 'Use React.createContext() with static contextType instead.', + 'LegacyFnConsumer uses the legacy contextTypes API which is no longer supported. ' + + 'Use React.createContext() with React.useContext() instead.', + ]); + expect(text).toBe( + '{}undefinedundefined', + ); + expect(lifecycleContextLog).toEqual([{}, {}, {}]); }); it('renders a tree with modern context', () => { diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index 302894091b86..973eba890d84 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -339,6 +339,42 @@ describe('ReactHooksWithNoopRenderer', () => { ); }); + it('dedupes the warning by component name', () => { + let _updateCountA; + function CounterA(props, ref) { + const [, updateCount] = useState(0); + _updateCountA = updateCount; + return null; + } + let _updateCountB; + function CounterB(props, ref) { + const [, updateCount] = useState(0); + _updateCountB = updateCount; + return null; + } + + ReactNoop.render([, ]); + expect(Scheduler).toFlushWithoutYielding(); + ReactNoop.render(null); + expect(Scheduler).toFlushWithoutYielding(); + expect(() => act(() => _updateCountA(1))).toErrorDev( + "Warning: Can't perform a React state update on an unmounted " + + 'component. This is a no-op, but it indicates a memory leak in your ' + + 'application. To fix, cancel all subscriptions and asynchronous ' + + 'tasks in a useEffect cleanup function.\n' + + ' in CounterA (at **)', + ); + // already cached so this logs no error + act(() => _updateCountA(2)); + expect(() => act(() => _updateCountB(1))).toErrorDev( + "Warning: Can't perform a React state update on an unmounted " + + 'component. This is a no-op, but it indicates a memory leak in your ' + + 'application. To fix, cancel all subscriptions and asynchronous ' + + 'tasks in a useEffect cleanup function.\n' + + ' in CounterB (at **)', + ); + }); + it('works with memo', () => { let _updateCount; function Counter(props) { diff --git a/packages/react/src/ReactElement.js b/packages/react/src/ReactElement.js index 9a0f208696d6..fc2302e7fde1 100644 --- a/packages/react/src/ReactElement.js +++ b/packages/react/src/ReactElement.js @@ -114,7 +114,7 @@ function warnIfStringRefCannotBeAutoConverted(config) { 'We ask you to manually fix this case by using useRef() or createRef() instead. ' + 'Learn more about using refs safely here: ' + 'https://fb.me/react-strict-mode-string-ref', - getComponentName(ReactCurrentOwner.current.type), + componentName, config.ref, ); didWarnAboutStringRefs[componentName] = true;