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" });

[bvaughn]

Oh that's a nice point, Dan. I had forgotten about that.

[main--]

Using Object.defineProperty changes the name used in the error message, but not in the component stack trace: https://codesandbox.io/s/jolly-sea-s7tl4

[bvaughn]

That's not the behavior I see in Chrome on OSX:

[main--]

Interesting! I can confirm that it works on Chromium (93.0.4577.63 Arch Linux) but breaks on Firefox (91.0.2 Arch Linux), where my screenshot was taken.

[main--]

Firefox (left) vs Chromium (right)

Seems like overriding Function.name using Object.defineProperty changes the name shown in native stack traces on Chromium but not on Firefox.

[main--]

Just to be clear, displayName is broken (at least for me) in all browsers. The problem looks like this: Native Component Stacks appear to just ignore displayName right now. Instead, it always prints the native function name. Using the Object.defineProperty hack, it is possible to override the native function name in some browsers (Chrome), but this approach seems very hacky and fragile. It also does not work with the library that's triggering my issue (react-easy-state) - not sure why, might be related to memoization.

[bvaughn]

Native Component Stacks appear to just ignore displayName right now.

PR #22477 may fix the issue being reported (without requiring the defineProperty hack).

[abrindam]

Looks like this issue has been inactive for awhile - if you'd prefer I open a new issue, let me know.

The following issue still exists:

Just to be clear, displayName is broken (at least for me) in all browsers. The problem looks like this: Native Component Stacks appear to just ignore displayName right now. Instead, it always prints the native function name.

From reading the underlying React Source code, this behavior appears to be intentional, with the idea being native component stacks will give you the most accurate information for debugging. I agree with this to a point, but as noted earlier in the thread, this is very problematic for Higher Order Components which want to preserve the name of the component they're wrapping.

In fact, this is such a primary use case that it is covered specifically in the React docs: https://reactjs.org/docs/higher-order-components.html#convention-wrap-the-display-name-for-easy-debugging

As it stands currently, that documentation is not accurate, because defining displayName doesn't do anything.

This is all independent of alternate mechanisms such as the previously mentioned defineProperty hack. Regardless of whether this hack works or not, the lack of support for displayName should be considered a bug, in my opinion, until the documentation is updated and/or a new convention is provided for how to make higher order components debuggable.

Accordingly, I would at least petition to drop "in Firefox" from this issue title, as the displayName issue is browser independent - it is only the workaround hack that is related to Firefox.

My personal suggestion is to override a portion of the stack frame with the displayName if it is set. This would require only a slight alteration to the code introduced by PR #22477 mentioned above, by adding the following else if clause:

                if (fn.displayName && _frame.includes('<anonymous>')) {
                  _frame = _frame.replace('<anonymous>', fn.displayName);
                } else if (fn.displayName) {
                  _frame = _frame.replace(fn.name, fn.displayName);
                }

I am not certain if this is robust enough for general use, but it solved the problem for me and is at least a step in the right direction.

[main--]

Given the age of this issue, it seems like it won't be fixed any time soon. However, I did find a workaround!

To recap, the problem is that given a snippet like this:

function higherOrderComponent(wrapped) {
  const wrappedComponent = (props) => {
    return wrapped(props);
  };

  return wrappedComponent
}

the closure will simply be named wrappedComponent and so the original name of the wrapped component (wrapped.name) is lost. While the most convenient solution here would undoubtedly be if React had a way to just override that name, we have to assume for now that this feature is not coming back.

So we need a way to set the actual name (and not just a shadowed "name" property like the hack for Chrome) of our closure to whatever wrapped.name says. It seems impossible - after all, the closure is named by the variable, and variable names are source code literals. So we can't just create a variable with a dynamic name, can we? In fact, we can do just that:

const wrappedComponent = { [wrapped.name](props) {
    return wrapped(props);
} }[wrapped.name];

Also works for class components:

const WrappedClassComp = { [Wrapped.name]: class extends Wrapped {
    // ...
} }[Wrapped.name];

Implemented in react-easy-state here: RisingStack/react-easy-state@458e4bb