Include component stack in invariants

[gaearon]

IIRC we didn't include it because it was DEV-only. But it's not anymore.
Maybe let's start including it?

Errors are often more prominent than warnings, and it would be great to have this info in both.

[Ethan-Arrowood]

I handled a similar issue #11208 before. Could I try this one? I'll try to find what these 'invariants' are.

[Ethan-Arrowood]

Sections of code like this?

[Ethan-Arrowood]

react/packages/react-dom/src/client/ReactDOM.js

Lines 657 to 710 in 7d27851

	function renderSubtreeIntoContainer(
	parentComponent: ?React$Component<any, any>,
	children: ReactNodeList,
	container: DOMContainer,
	forceHydrate: boolean,
	callback: ?Function,
	) {
	invariant(
	isValidContainer(container),
	'Target container is not a DOM element.',
	);
	if (__DEV__) {
	if (container._reactRootContainer && container.nodeType !== COMMENT_NODE) {
	const hostInstance = DOMRenderer.findHostInstanceWithNoPortals(
	container._reactRootContainer.current,
	);
	if (hostInstance) {
	warning(
	hostInstance.parentNode === container,
	'render(...): It looks like the React-rendered content of this ' +
	'container was removed without using React. This is not ' +
	'supported and will cause errors. Instead, call ' +
	'ReactDOM.unmountComponentAtNode to empty a container.',
	);
	}
	}
	const isRootRenderedBySomeReact = !!container._reactRootContainer;
	const rootEl = getReactRootElementInContainer(container);
	const hasNonRootReactChild = !!(
	rootEl && ReactDOMComponentTree.getInstanceFromNode(rootEl)
	);
	warning(
	!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(
	container.nodeType !== ELEMENT_NODE ||
	!((container: any): Element).tagName ||
	((container: any): Element).tagName.toUpperCase() !== 'BODY',
	'render(): Rendering components directly into document.body is ' +
	'discouraged, since its children are often manipulated by third-party ' +
	'scripts and browser extensions. This may lead to subtle ' +
	'reconciliation issues. Try rendering into a container element created ' +
	'for your app.',
	);
	}

Using the code from my previous issue:

var {getCurrentFiberStackAddendum} = require('ReactDebugCurrentFiber');
invariant(
    isValidContainer(container),
    'Target container is not a DOM element.%s',
    getCurrentFiberStackAddendum() || ''
);

Would I also want to add the getCurrentFiberStackAddendum() || '' to the warnings on lines 691 & 699?

[gaearon]

This is top level rendering so component stack wouldn't be useful there (you're not inside a component). I was thinking about these:

react/packages/react-reconciler/src/ReactChildFiber.js

Lines 162 to 164 in 7d27851

	invariant(
	false,
	'Objects are not valid as a React child (found: %s).%s',

react/packages/react-reconciler/src/ReactFiber.js

Lines 342 to 346 in 7d27851

	invariant(
	false,
	'Element type is invalid: expected a string (for built-in components) ' +
	'or a class/function (for composite components) but got: %s.%s',
	type == null ? type : typeof type,

[Ethan-Arrowood]

Okay sweet. I'm down to implement the component stack in each of these. What other files should I implement this change in? Is there an easy way to determine which are inside a component? Should I implement the component stack in every invariant call in these files?

[gaearon]

Is there an easy way to determine which are inside a component?

Look at what the message says. Look at examples of this invariant in tests. See if it's something that happens inside a component or not.

[Ethan-Arrowood]

Gotcha. I noticed some invariants already have the component stack on the __DEV__ addendum (Lines 157 and 168):

react/packages/react-reconciler/src/ReactChildFiber.js

Lines 153 to 171 in 7d27851

	function throwOnInvalidObjectType(returnFiber: Fiber, newChild: Object) {
	if (returnFiber.type !== 'textarea') {
	let addendum = '';
	if (__DEV__) {
	addendum =
	' If you meant to render a collection of children, use an array ' +
	'instead.' +
	(getCurrentFiberStackAddendum() || '');
	}
	invariant(
	false,
	'Objects are not valid as a React child (found: %s).%s',
	Object.prototype.toString.call(newChild) === '[object Object]'
	? 'object with keys {' + Object.keys(newChild).join(', ') + '}'
	: newChild,
	addendum,
	);
	}
	}

Should I remove it from the __DEV__ and put it directly on the invariant:

invariant(
      false,
      'Objects are not valid as a React child (found: %s).%s.%s',
      Object.prototype.toString.call(newChild) === '[object Object]'
        ? 'object with keys {' + Object.keys(newChild).join(', ') + '}'
        : newChild,
      addendum,
      getCurrentFiberStackAddendum() || '',
 );

[gaearon]

Should I remove it from the DEV and put it directly on the invariant:

That sounds reasonable.

[bl00mber]

Currently getCurrentFiberStackInDev() is DEV-only 😯
Is this issue still relevant?

[sophiebits]

We already log the component stack to the console along with an error (see discussion on #18315). So I don't think there is any action required now. (@gaearon lmk if I'm missing something)

[gaearon]

Yeah that makes sense. I think our integrations like CRA error box don't currently surface that but it's something that can be addressed in them.