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

Add component stacks to (almost) all warnings #17586

Merged
merged 1 commit into from Dec 12, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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