Skip to content

Commit

Permalink
Add component stacks to (almost) all warnings (#17586)
Browse files Browse the repository at this point in the history
  • Loading branch information
gaearon committed Dec 12, 2019
1 parent 612a768 commit b15bf36
Show file tree
Hide file tree
Showing 83 changed files with 349 additions and 513 deletions.
8 changes: 3 additions & 5 deletions packages/create-subscription/src/createSubscription.js
Expand Up @@ -9,7 +9,7 @@

import React from 'react';
import invariant from 'shared/invariant';
import warningWithoutStack from 'shared/warningWithoutStack';
import warning from 'shared/warning';

type Unsubscribe = () => void;

Expand Down Expand Up @@ -38,12 +38,10 @@ export function createSubscription<Property, Value>(

if (__DEV__) {
if (typeof getCurrentValue !== 'function') {
warningWithoutStack(
'Subscription must specify a getCurrentValue function',
);
warning('Subscription must specify a getCurrentValue function');
}
if (typeof subscribe !== 'function') {
warningWithoutStack('Subscription must specify a subscribe function');
warning('Subscription must specify a subscribe function');
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/legacy-events/EventPluginUtils.js
Expand Up @@ -7,7 +7,7 @@

import {invokeGuardedCallbackAndCatchFirstError} from 'shared/ReactErrorUtils';
import invariant from 'shared/invariant';
import warningWithoutStack from 'shared/warningWithoutStack';
import warning from 'shared/warning';

export let getFiberCurrentPropsFromNode = null;
export let getInstanceFromNode = null;
Expand All @@ -23,7 +23,7 @@ export function setComponentTree(
getNodeFromInstance = getNodeFromInstanceImpl;
if (__DEV__) {
if (!getNodeFromInstance || !getInstanceFromNode) {
warningWithoutStack(
warning(
'EventPluginUtils.setComponentTree(...): Injected ' +
'module is missing getNodeFromInstance or getInstanceFromNode.',
);
Expand Down Expand Up @@ -52,7 +52,7 @@ if (__DEV__) {
: 0;

if (instancesIsArr !== listenersIsArr || instancesLen !== listenersLen) {
warningWithoutStack('EventPluginUtils: Invalid `event`.');
warning('EventPluginUtils: Invalid `event`.');
}
};
}
Expand Down
4 changes: 2 additions & 2 deletions packages/legacy-events/EventPropagators.js
Expand Up @@ -10,7 +10,7 @@ import {
traverseTwoPhase,
traverseEnterLeave,
} from 'shared/ReactTreeTraversal';
import warningWithoutStack from 'shared/warningWithoutStack';
import warning from 'shared/warning';

import {getListener} from './EventPluginHub';
import accumulateInto from './accumulateInto';
Expand Down Expand Up @@ -47,7 +47,7 @@ function listenerAtPhase(inst, event, propagationPhase: PropagationPhases) {
function accumulateDirectionalDispatches(inst, phase, event) {
if (__DEV__) {
if (!inst) {
warningWithoutStack('Dispatching inst must not be null');
warning('Dispatching inst must not be null');
}
}
const listener = listenerAtPhase(inst, event, phase);
Expand Down
6 changes: 3 additions & 3 deletions packages/legacy-events/ResponderTouchHistoryStore.js
Expand Up @@ -8,7 +8,7 @@
*/

import invariant from 'shared/invariant';
import warningWithoutStack from 'shared/warningWithoutStack';
import warning from 'shared/warning';

import {isStartish, isMoveish, isEndish} from './ResponderTopLevelEventTypes';

Expand Down Expand Up @@ -96,7 +96,7 @@ function getTouchIdentifier({identifier}: Touch): number {
invariant(identifier != null, 'Touch object is missing identifier.');
if (__DEV__) {
if (identifier > MAX_TOUCH_BANK) {
warningWithoutStack(
warning(
'Touch identifier %s is greater than maximum supported %s which causes ' +
'performance issues backfilling array locations for all of the indices.',
identifier,
Expand Down Expand Up @@ -202,7 +202,7 @@ const ResponderTouchHistoryStore = {
if (__DEV__) {
const activeRecord = touchBank[touchHistory.indexOfSingleActiveTouch];
if (activeRecord == null || !activeRecord.touchActive) {
warningWithoutStack('Cannot find single active touch.');
warning('Cannot find single active touch.');
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/legacy-events/SyntheticEvent.js
Expand Up @@ -8,7 +8,7 @@
/* eslint valid-typeof: 0 */

import invariant from 'shared/invariant';
import warningWithoutStack from 'shared/warningWithoutStack';
import warning from 'shared/warning';

const EVENT_POOL_SIZE = 10;

Expand Down Expand Up @@ -284,7 +284,7 @@ function getPooledWarningPropertyDefinition(propName, getVal) {

function warn(action, result) {
if (__DEV__) {
warningWithoutStack(
warning(
"This synthetic event is reused for performance reasons. If you're seeing this, " +
"you're %s `%s` on a released/nullified synthetic event. %s. " +
'If you must keep the original synthetic event around, use event.persist(). ' +
Expand Down
4 changes: 2 additions & 2 deletions packages/react-cache/src/ReactCache.js
Expand Up @@ -8,7 +8,7 @@
*/

import React from 'react';
import warningWithoutStack from 'shared/warningWithoutStack';
import warning from 'shared/warning';

import {createLRU} from './LRU';

Expand Down Expand Up @@ -71,7 +71,7 @@ function identityHashFn(input) {
input !== undefined &&
input !== null
) {
warningWithoutStack(
warning(
'Invalid key type. Expected a string, number, symbol, or boolean, ' +
'but instead received: %s' +
'\n\nTo use non-primitive values as keys, you must pass a hash ' +
Expand Down
15 changes: 6 additions & 9 deletions packages/react-cache/src/__tests__/ReactCache-test.internal.js
Expand Up @@ -172,15 +172,12 @@ describe('ReactCache', () => {
if (__DEV__) {
expect(() => {
expect(Scheduler).toFlushAndYield(['App', 'Loading...']);
}).toWarnDev(
[
'Invalid key type. Expected a string, number, symbol, or ' +
'boolean, but instead received: Hi,100\n\n' +
'To use non-primitive values as keys, you must pass a hash ' +
'function as the second argument to createResource().',
],
{withoutStack: true},
);
}).toWarnDev([
'Invalid key type. Expected a string, number, symbol, or ' +
'boolean, but instead received: Hi,100\n\n' +
'To use non-primitive values as keys, you must pass a hash ' +
'function as the second argument to createResource().',
]);
} else {
expect(Scheduler).toFlushAndYield(['App', 'Loading...']);
}
Expand Down
31 changes: 3 additions & 28 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
Expand Up @@ -215,7 +215,6 @@ describe('ReactComponentLifeCycle', () => {
'StatefulComponent: It is not recommended to assign props directly to state ' +
"because updates to props won't be reflected in state. " +
'In most cases, it is better to use props directly.',
{withoutStack: true},
);
});

Expand All @@ -240,7 +239,6 @@ describe('ReactComponentLifeCycle', () => {
'This is a no-op, but it might indicate a bug in your application. ' +
'Instead, assign to `this.state` directly or define a `state = {};` ' +
'class property with the desired state in the StatefulComponent component.',
{withoutStack: true},
);

// Check deduplication; (no extra warnings should be logged).
Expand Down Expand Up @@ -271,9 +269,7 @@ describe('ReactComponentLifeCycle', () => {
expect(() => {
const instance = ReactTestUtils.renderIntoDocument(element);
expect(instance._isMounted()).toBeTruthy();
}).toWarnDev('Component is accessing isMounted inside its render()', {
withoutStack: true,
});
}).toWarnDev('Component is accessing isMounted inside its render()');
});

it('should correctly determine if a null component is mounted', () => {
Expand All @@ -300,9 +296,7 @@ describe('ReactComponentLifeCycle', () => {
expect(() => {
const instance = ReactTestUtils.renderIntoDocument(element);
expect(instance._isMounted()).toBeTruthy();
}).toWarnDev('Component is accessing isMounted inside its render()', {
withoutStack: true,
});
}).toWarnDev('Component is accessing isMounted inside its render()');
});

it('isMounted should return false when unmounted', () => {
Expand Down Expand Up @@ -340,9 +334,7 @@ describe('ReactComponentLifeCycle', () => {

expect(() => {
ReactTestUtils.renderIntoDocument(<Component />);
}).toWarnDev('Component is accessing findDOMNode inside its render()', {
withoutStack: true,
});
}).toWarnDev('Component is accessing findDOMNode inside its render()');
});

it('should carry through each of the phases of setup', () => {
Expand Down Expand Up @@ -408,7 +400,6 @@ describe('ReactComponentLifeCycle', () => {
instance = ReactDOM.render(<LifeCycleComponent />, container);
}).toWarnDev(
'LifeCycleComponent is accessing isMounted inside its render() function',
{withoutStack: true},
);

// getInitialState
Expand Down Expand Up @@ -705,7 +696,6 @@ describe('ReactComponentLifeCycle', () => {
expect(() => {
expect(() => ReactDOM.render(<Component />, container)).toWarnDev(
'Unsafe legacy lifecycles will not be called for components using new component APIs.',
{withoutStack: true},
);
}).toLowPriorityWarnDev(
[
Expand Down Expand Up @@ -744,7 +734,6 @@ describe('ReactComponentLifeCycle', () => {
ReactDOM.render(<Component value={1} />, container),
).toWarnDev(
'Unsafe legacy lifecycles will not be called for components using new component APIs.',
{withoutStack: true},
);
}).toLowPriorityWarnDev(
[
Expand Down Expand Up @@ -780,7 +769,6 @@ describe('ReactComponentLifeCycle', () => {
const container = document.createElement('div');
expect(() => ReactDOM.render(<Component value={1} />, container)).toWarnDev(
'Unsafe legacy lifecycles will not be called for components using new component APIs.',
{withoutStack: true},
);
ReactDOM.render(<Component value={2} />, container);
});
Expand Down Expand Up @@ -812,7 +800,6 @@ describe('ReactComponentLifeCycle', () => {
' componentWillUpdate\n\n' +
'The above lifecycles should be removed. Learn more about this warning here:\n' +
'https://fb.me/react-unsafe-component-lifecycles',
{withoutStack: true},
);
}).toLowPriorityWarnDev(
[
Expand All @@ -839,7 +826,6 @@ describe('ReactComponentLifeCycle', () => {
' UNSAFE_componentWillMount\n\n' +
'The above lifecycles should be removed. Learn more about this warning here:\n' +
'https://fb.me/react-unsafe-component-lifecycles',
{withoutStack: true},
);

class WillMountAndUpdate extends React.Component {
Expand All @@ -864,7 +850,6 @@ describe('ReactComponentLifeCycle', () => {
' UNSAFE_componentWillUpdate\n\n' +
'The above lifecycles should be removed. Learn more about this warning here:\n' +
'https://fb.me/react-unsafe-component-lifecycles',
{withoutStack: true},
);
}).toLowPriorityWarnDev(['componentWillMount has been renamed'], {
withoutStack: true,
Expand All @@ -888,7 +873,6 @@ describe('ReactComponentLifeCycle', () => {
' componentWillReceiveProps\n\n' +
'The above lifecycles should be removed. Learn more about this warning here:\n' +
'https://fb.me/react-unsafe-component-lifecycles',
{withoutStack: true},
);
}).toLowPriorityWarnDev(['componentWillReceiveProps has been renamed'], {
withoutStack: true,
Expand Down Expand Up @@ -921,7 +905,6 @@ describe('ReactComponentLifeCycle', () => {
' componentWillUpdate\n\n' +
'The above lifecycles should be removed. Learn more about this warning here:\n' +
'https://fb.me/react-unsafe-component-lifecycles',
{withoutStack: true},
);
}).toLowPriorityWarnDev(
[
Expand All @@ -947,7 +930,6 @@ describe('ReactComponentLifeCycle', () => {
' UNSAFE_componentWillMount\n\n' +
'The above lifecycles should be removed. Learn more about this warning here:\n' +
'https://fb.me/react-unsafe-component-lifecycles',
{withoutStack: true},
);

class WillMountAndUpdate extends React.Component {
Expand All @@ -971,7 +953,6 @@ describe('ReactComponentLifeCycle', () => {
' UNSAFE_componentWillUpdate\n\n' +
'The above lifecycles should be removed. Learn more about this warning here:\n' +
'https://fb.me/react-unsafe-component-lifecycles',
{withoutStack: true},
);
}).toLowPriorityWarnDev(['componentWillMount has been renamed'], {
withoutStack: true,
Expand All @@ -994,7 +975,6 @@ describe('ReactComponentLifeCycle', () => {
' componentWillReceiveProps\n\n' +
'The above lifecycles should be removed. Learn more about this warning here:\n' +
'https://fb.me/react-unsafe-component-lifecycles',
{withoutStack: true},
);
}).toLowPriorityWarnDev(['componentWillReceiveProps has been renamed'], {
withoutStack: true,
Expand Down Expand Up @@ -1045,7 +1025,6 @@ describe('ReactComponentLifeCycle', () => {
"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.",
{withoutStack: true},
);
ReactDOM.render(<Parent ref={c => c && log.push('ref')} />, div);

Expand Down Expand Up @@ -1074,7 +1053,6 @@ describe('ReactComponentLifeCycle', () => {
expect(() => ReactDOM.render(<MyComponent />, div)).toWarnDev(
'MyComponent.getDerivedStateFromProps(): A valid state object (or null) must ' +
'be returned. You have returned undefined.',
{withoutStack: true},
);

// De-duped
Expand All @@ -1097,7 +1075,6 @@ describe('ReactComponentLifeCycle', () => {
'undefined. This is not recommended. Instead, define the initial state by ' +
'assigning an object to `this.state` in the constructor of `MyComponent`. ' +
'This ensures that `getDerivedStateFromProps` arguments have a consistent shape.',
{withoutStack: true},
);

// De-duped
Expand Down Expand Up @@ -1366,7 +1343,6 @@ describe('ReactComponentLifeCycle', () => {
expect(() => ReactDOM.render(<MyComponent value="bar" />, div)).toWarnDev(
'MyComponent.getSnapshotBeforeUpdate(): A snapshot value (or null) must ' +
'be returned. You have returned undefined.',
{withoutStack: true},
);

// De-duped
Expand All @@ -1387,7 +1363,6 @@ describe('ReactComponentLifeCycle', () => {
expect(() => ReactDOM.render(<MyComponent />, div)).toWarnDev(
'MyComponent: getSnapshotBeforeUpdate() should be used with componentDidUpdate(). ' +
'This component defines getSnapshotBeforeUpdate() only.',
{withoutStack: true},
);

// De-duped
Expand Down

0 comments on commit b15bf36

Please sign in to comment.