Skip to content

Commit

Permalink
Change warning() to automatically inject the stack, and add warningWi…
Browse files Browse the repository at this point in the history
…thoutStack() as opt-out (facebook#13161)

* Use %s in the console calls

* Add shared/warningWithStack

* Convert some warning callsites to warningWithStack

* Use warningInStack in shared utilities and remove unnecessary checks

* Replace more warning() calls with warningWithStack()

* Fixes after rebase + use warningWithStack in react

* Make warning have stack by default; warningWithoutStack opts out

* Forbid builds that may not use internals

* Revert newly added stacks

I changed my mind and want to keep this PR without functional changes. So we won't "fix" any warnings that are already missing stacks. We'll do it in follow-ups instead.

* Fix silly find/replace mistake

* Reorder imports

* Add protection against warning argument count mismatches

* Address review
  • Loading branch information
gaearon authored Jul 16, 2018
1 parent 854c953 commit f9358c5
Show file tree
Hide file tree
Showing 59 changed files with 426 additions and 390 deletions.
6 changes: 3 additions & 3 deletions packages/create-subscription/src/createSubscription.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

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

type Unsubscribe = () => void;

Expand All @@ -36,11 +36,11 @@ export function createSubscription<Property, Value>(
}> {
const {getCurrentValue, subscribe} = config;

warning(
warningWithoutStack(
typeof getCurrentValue === 'function',
'Subscription must specify a getCurrentValue function',
);
warning(
warningWithoutStack(
typeof subscribe === 'function',
'Subscription must specify a subscribe function',
);
Expand Down
6 changes: 3 additions & 3 deletions packages/events/EventPluginUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

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

export let getFiberCurrentPropsFromNode = null;
export let getInstanceFromNode = null;
Expand All @@ -21,7 +21,7 @@ export const injection = {
getNodeFromInstance,
} = Injected);
if (__DEV__) {
warning(
warningWithoutStack(
getNodeFromInstance && getInstanceFromNode,
'EventPluginUtils.injection.injectComponentTree(...): Injected ' +
'module is missing getNodeFromInstance or getInstanceFromNode.',
Expand All @@ -46,7 +46,7 @@ if (__DEV__) {
? dispatchInstances.length
: dispatchInstances ? 1 : 0;

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

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

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

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

Expand Down Expand Up @@ -95,7 +95,7 @@ function resetTouchRecord(touchRecord: TouchRecord, touch: Touch): void {
function getTouchIdentifier({identifier}: Touch): number {
invariant(identifier != null, 'Touch object is missing identifier.');
if (__DEV__) {
warning(
warningWithoutStack(
identifier <= MAX_TOUCH_BANK,
'Touch identifier %s is greater than maximum supported %s which causes ' +
'performance issues backfilling array locations for all of the indices.',
Expand Down Expand Up @@ -200,7 +200,7 @@ const ResponderTouchHistoryStore = {
}
if (__DEV__) {
const activeRecord = touchBank[touchHistory.indexOfSingleActiveTouch];
warning(
warningWithoutStack(
activeRecord != null && activeRecord.touchActive,
'Cannot find single active touch.',
);
Expand Down
6 changes: 3 additions & 3 deletions packages/events/SyntheticEvent.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/* eslint valid-typeof: 0 */

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

let didWarnForAddedNewProperty = false;
const EVENT_POOL_SIZE = 10;
Expand Down Expand Up @@ -261,7 +261,7 @@ if (__DEV__) {
!target.constructor.Interface.hasOwnProperty(prop) &&
shouldBeReleasedProperties.indexOf(prop) === -1
) {
warning(
warningWithoutStack(
didWarnForAddedNewProperty || target.isPersistent(),
"This synthetic event is reused for performance reasons. If you're " +
"seeing this, you're adding a new property in the synthetic event object. " +
Expand Down Expand Up @@ -316,7 +316,7 @@ function getPooledWarningPropertyDefinition(propName, getVal) {

function warn(action, result) {
const warningCondition = false;
warning(
warningWithoutStack(
warningCondition,
"This synthetic event is reused for performance reasons. If you're seeing this, " +
"you're %s `%s` on a released/nullified synthetic event. %s. " +
Expand Down
20 changes: 10 additions & 10 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import ReactSharedInternals from 'shared/ReactSharedInternals';
import getComponentName from 'shared/getComponentName';
import invariant from 'shared/invariant';
import lowPriorityWarning from 'shared/lowPriorityWarning';
import warning from 'shared/warning';
import warningWithoutStack from 'shared/warningWithoutStack';

import * as ReactDOMComponentTree from './ReactDOMComponentTree';
import * as ReactDOMFiberComponent from './ReactDOMFiberComponent';
Expand Down Expand Up @@ -64,7 +64,7 @@ if (__DEV__) {
typeof Set.prototype.clear !== 'function' ||
typeof Set.prototype.forEach !== 'function'
) {
warning(
warningWithoutStack(
false,
'React depends on Map and Set built-in types. Make sure that you load a ' +
'polyfill in older browsers. https://fb.me/react-polyfills',
Expand All @@ -77,7 +77,7 @@ if (__DEV__) {
container._reactRootContainer._internalRoot.current,
);
if (hostInstance) {
warning(
warningWithoutStack(
hostInstance.parentNode === container,
'render(...): It looks like the React-rendered content of this ' +
'container was removed without using React. This is not ' +
Expand All @@ -93,15 +93,15 @@ if (__DEV__) {
rootEl && ReactDOMComponentTree.getInstanceFromNode(rootEl)
);

warning(
warningWithoutStack(
!hasNonRootReactChild || isRootRenderedBySomeReact,
'render(...): Replacing React-rendered children with a new root ' +
'component. If you intended to update the children of this node, ' +
'you should instead have the existing children update their state ' +
'and render the new components instead of calling ReactDOM.render.',
);

warning(
warningWithoutStack(
container.nodeType !== ELEMENT_NODE ||
!((container: any): Element).tagName ||
((container: any): Element).tagName.toUpperCase() !== 'BODY',
Expand All @@ -114,7 +114,7 @@ if (__DEV__) {
};

warnOnInvalidCallback = function(callback: mixed, callerName: string) {
warning(
warningWithoutStack(
callback === null || typeof callback === 'function',
'%s(...): Expected the last optional `callback` argument to be a ' +
'function. Instead received: %s.',
Expand Down Expand Up @@ -472,7 +472,7 @@ function legacyCreateRootFromDOMContainer(
(rootSibling: any).hasAttribute(ROOT_ATTRIBUTE_NAME)
) {
warned = true;
warning(
warningWithoutStack(
false,
'render(): Target node has markup rendered by React, but there ' +
'are unrelated nodes as well. This is most commonly caused by ' +
Expand Down Expand Up @@ -590,7 +590,7 @@ const ReactDOM: Object = {
if (owner !== null && owner.stateNode !== null) {
const warnedAboutRefsInRender =
owner.stateNode._warnedAboutRefsInRender;
warning(
warningWithoutStack(
warnedAboutRefsInRender,
'%s is accessing findDOMNode inside its render(). ' +
'render() should be a pure function of props and state. It should ' +
Expand Down Expand Up @@ -667,7 +667,7 @@ const ReactDOM: Object = {
const rootEl = getReactRootElementInContainer(container);
const renderedByDifferentReact =
rootEl && !ReactDOMComponentTree.getInstanceFromNode(rootEl);
warning(
warningWithoutStack(
!renderedByDifferentReact,
"unmountComponentAtNode(): The node you're attempting to unmount " +
'was rendered by another copy of React.',
Expand Down Expand Up @@ -696,7 +696,7 @@ const ReactDOM: Object = {
isValidContainer(container.parentNode) &&
!!container.parentNode._reactRootContainer;

warning(
warningWithoutStack(
!hasNonRootReactChild,
"unmountComponentAtNode(): The node you're attempting to unmount " +
'was rendered by React and is not a top-level container. %s',
Expand Down
Loading

0 comments on commit f9358c5

Please sign in to comment.