Bug: Native Component Stacks don't respect function "displayName" in Firefox

[main--]

Edit See this comment for the key part of what's being reported/discussed on this issue: #22315 (comment)

Original bug report below

As of #18561 component stacks are generated from native stack frames. This is problematic with HOCs that inherit from the input component in order to change its behavior. The somewhat popular @risingstack/react-easy-state package is one example of such a component. While it does assign a displayName, the new Native Component Stacks appear to ignore this. Instead, components wrapped in view() (from react-easy-state) are always shown with the name of the wrapper class, i.e., ReactiveComp or ReactiveClassComp.

This is especially catastrophic in the case of react-easy-state, where one is supposed to wrap essentially all components in the entire codebase in the view() HOC. The result is that component stacks become unusable for debugging.

Is there perhaps a way to work around this (e.g. disable native component stacks, or some new way to explicitly provide a component name like displayName)?

React version: 17.0.1

Steps To Reproduce

1. Apply a HOC that uses inheritance (i.e., inherits from the component instead of wrapping it in JSX) to a component.
2. The component will always be named ReactiveComp or ReactiveClassComp in component stack traces.

Link to code example: https://codesandbox.io/s/rough-tdd-wqepe?file=/src/App.tsx

The current behavior

Something went wrong.
Error: sorry

BadGuy@https://7ww9j.csb.app/src/App.tsx:21:9
ErrorBoundary@https://7ww9j.csb.app/src/App.tsx:33:5
div
App

Something went wrong.
Error: sorry

ReactiveComp@https://7ww9j.csb.app/node_modules/@risingstack/react-easy-state/dist/es.es6.js:62:53
ErrorBoundary@https://7ww9j.csb.app/src/App.tsx:33:5
div
App

Note how the component wrapped in view() is shown as ReactiveComp instead of either the function name or the explicitly assigned displayName.

The expected behavior

The name of the ReactiveComp wrapper should never appear in component stacks.

[bvaughn]

Note how the component wrapped in view() is shown as ReactiveComp instead of either the function name or the explicitly assigned displayName.

Just to be clear, ignoring the displayName attribute is (I'm pretty sure) an intentional part of the "native component stacks" - which are supposed to exactly mimic the format of native stacks (Error().stack) which also don't respect displayName. The major benefits of this approach are:

1. Doesn't require/rely on shipping displayName in production builds.
2. Component stacks can be processed in the same way as native stacks (since they share the same format).

I appreciate the repro! Would it be possible to get one that doesn't include any external packages (like react-easy-state)? (It's always nice for the repro to be as small and self-contained as possible.)

Digging into the third party library you mentioned, I don't think the problem is actually "inheritance" at all but rather the way the component is being rendered in the HOC:

      // create a memoized reactive wrapper of the original component (render)
      // at the very first run of the component function
      const render = useMemo(
        () =>
          observe(Comp, {
            scheduler: () => setState({}),
            lazy: true,
          }),
        // Adding the original Comp here is necessary to make React Hot Reload work
        // it does not affect behavior otherwise
        [Comp],
      );

For example, this simplified repro shows the behavior you describe:
https://codesandbox.io/s/condescending-cartwright-qn8c2?file=/src/App.tsx

But this one does not:
https://codesandbox.io/s/exciting-pasteur-p98fn?file=/src/App.tsx

That's because the latter approach is not actually rendering Comp as a React element but rather as a "render prop", which breaks our stack detection logic (since "render props" aren't React elements).

[main--]

Thank you for the fast response!

Digging into the third party library you mentioned, I don't think the problem is actually "inheritance" at all but rather the way the component is being rendered in the HOC:

I think you're spot on that this is about render prop vs React element. Sorry, I confused the terminology.

So it looks like moving away from render props could be a workaround for now. I'm not sure if this is actually possible for react-easy-state though, but I can try.

One unfortunate aspect about this issue is that the docs still imply that displayName can be used to specify what name should be used in component stack traces. Since displayName used to override the inferred function name previously (and is still respected by the React Developer Tools extension), I think that ignoring it for component stack traces may be a bug.

[bvaughn]

I think that ignoring it for component stack traces may be a bug.

Can you share a reduced repro that shows this behavior specifically?

Looking at the code, it seems like the displayName attribute should actually be respected (at least in DEV builds):

react/packages/shared/ReactComponentStackFrame.js

Lines 260 to 262 in 50263d3

	if (__DEV__ && ownerFn) {
	ownerName = ownerFn.displayName || ownerFn.name || null;
	}

[main--]

Can you share a reduced repro that shows this behavior specifically?

Here you go: https://codesandbox.io/s/naughty-austin-jk28f?file=/src/App.tsx

It's basically the renderprop example you linked above, but instead of react-error-boundary I'm using a custom ErrorBoundary component (based on example code from somewhere in the docs).

We still have the two components BadGuy and WrappedBadGuy, the latter of which is wrapped in the dummy view() HOC from your example that just passes the component as a render prop. For WrappedBadGuy we have both an underlying function name (_WrappedBadGuy) as well as an explicitly assigned displayName (WrappedBadGuyDisplayName). But still, the errorInfo.componentStack just shows the second component as ReactiveComp.

[bvaughn]

Ah, sorry. To be clear that is the expected behavior for the ReactiveComp because WrappedBadGuy is not being called as a React element but rather as a regular function (so React doesn't know anything about it / can't access / can't see it to read the displayName prop).

[main--]

I think there is a miscommunication here. WrappedBadGuy is in fact called as a React element below:

      <ErrorBoundary>
        <WrappedBadGuy />
      </ErrorBoundary>

What's being passed as a render prop is the inner function _WrappedBadGuy (perhaps I should have picked a different name to make the difference clearer). But the displayName is being assigned directly to the element, and not the inner function:

function view(Comp: any) {
  let ReactiveComp = (props: any) => {
    return Comp(props);
  };

  return ReactiveComp;
}

const WrappedBadGuy = view(function _WrappedBadGuy() {
  throw Error("sorry");
});
(WrappedBadGuy as any).displayName = "WrappedBadGuyDisplayName";

[bvaughn]

Ah, you are correct. I missed this line:

(WrappedBadGuy as any).displayName = "WrappedBadGuyDisplayName";

I don't think I see how the view() wrapper is relevant to this repro then 😄

Seems like what you're really reporting is that a component like this:

function BadGuy() {
  throw Error("sorry");
}
BadGuy.displayName = 'BadGuyDisplayName';

Will show up as BadGuy in a component stack rather than BadGuyDisplayName. Is that right?

In other words, it will show this:

The above error occurred in the <BadGuyDisplayName> component:
    at BadGuy (https://jk28f.csb.app/src/App.tsx:27:9)
    at ErrorBoundary 
    ...

Here is a reduced repro:
https://codesandbox.io/s/festive-ritchie-webcl?file=/src/App.tsx

[main--]

Somehow it never occurred to me to test this behavior without some kind of HOC, but I think it really is that simple! 😄

[bvaughn]

Okay! Glad we're on the same page. :)

I'm on-call for DevTools this week so I don't really have the bandwidth to dig in any further, but I think we have enough info now for this to be discussed/decided.

I'm not positive but I don't think what you're looking for is possible (reading the displayName for the "native stack") because we use Error to construct these stacks (so they'll be in the exact same format) and Error doesn't care about the displayName convention.

[gaearon]

You can Object.defineProperty instead for name. This works: https://codesandbox.io/s/tender-easley-vck0q?file=/src/App.tsx

function BadGuy() {
  throw Error("sorry");
}
Object.defineProperty(BadGuy, "name", { value: "BadGuyDisplayName" });