Component with forwardRef passed as prop

[tu4mo]

const A = () => <div />
const B = React.forwardRef(() => <div />)

What I get:

<Comp
  a={A}
  b={{
    $$typeof: Symbol(react.forward_ref),
    render: _c3
  }}
/>

What I expect:

<Comp
  a={A}
  b={B}
/>

Repro: https://codesandbox.io/s/dank-cdn-ftvik?file=/src/App.js

[armandabric]

Thanks to #617 we are now supporting React.forwardRef(). But I still reproduce your issue with this test:

it('should use inferred function name as display name for `forwardRef` element when used in a props', () => {
    const Comp = () => <div />;
    const A = () => <div />;
    const B = React.forwardRef(function B(_props, ref) {
      return <div ref={ref} />;
    });

    expect(reactElementToJSXString(<Comp a={A} b={B}></Comp>)).toEqual(
      `<Comp
  a={function noRefCheck() {}}
  // Should be <B />
  b={{
    $$typeof: Symbol(react.forward_ref),
    render: function noRefCheck() {}
  }}
/>`
    );
  });

As it's fresh in it's mind maybe @Andarist have an idea?

[Andarist]

It's an interesting case - this is not a valid React element so it doesn't go into this logic: https://github.com/algolia/react-element-to-jsx-string/blob/2c5132e58f69414d7b67e24cd470594656261b7f/src/formatter/formatPropValue.js#L47-L54

A valid React element is a result of the React.createElement call (so basically the output of JSX: b={<B/>}). This is a component type~, like a class, function, result of the React.memo and more.

[armandabric]

Good point. I'm not sure what we could propose as in fact B is only an object parsed to a prop: not a react element.

[Andarist]

Well, it could still be formatted somehow - since we can recognize that it's not "just an object". But I don't believe printing <Comp/> is OK here. The suggested expected formatting could work OK here - printing Comp in this case. Note that in the given example we would not be able to print B because everything is anonymous there, but with inferred names and display names something nicer could be printed.

I probably won't take this work on myself though - it shouldn't be overly hard though with the groundwork being laid out already.

[armandabric]

I agree with you we could easily add a dedicated render for this use case but I lack of idea on the shape of this render.

If people really need this they could explain us their use case and make us proposal. If it's a reasonable use case we will consider to handle this. I keep this issue open for the discussion.

[tu4mo]

My exact use case is documenting polymorphic components with as prop.

<Link as={RouterLink}>Link</Link>

If the as component is wrapped with a forwardRef (like they usually are) it does not work.

[armandabric]

I see. We could maybe have a dedicated behavior for the as props. It's a common usage

[Andarist]

OTOH - this really could be handled in the same way for any prop. People probably rarely want to see the react element's internal structure in their stringified trees

[iMoses-Apiiro]

I think we have two cases here to handle:

• a prop which is of type object and has a $$typeof property of type symbol, in which we can recursively search for a displayName
• a prop which is of type function that has a displayName, or fallback to name

The first case can be solved like this:

// formatPropValue.js.flow:43

  if (typeof propValue.$$typeof === 'symbol') {
    return `{${parseReactElement(createElement(propValue), options).displayName}}`;
  }

Or we can export getReactElementDisplayName so we can bypass parseReactElement and get the displayName directly

Regarding function components, what about this method? Could also improve regular functions display by using the name as fallback before noRefCheck

// formatFunction.js.flow:16

const getFucntionDisplayName = (fn: Function): string | undefined =>
  fn.displayName || fn.name;

export default (fn: Function, options: Options): string => {
  const {
    showFunctions,
    functionValue = defaultFunctionValue,
    displayName: displayNameFn = getFucntionDisplayName,
  } = options;
  if (!showFunctions && functionValue === defaultFunctionValue) {
    return displayNameFn(fn) || defaultFunctionValue(noRefCheck);
  }

  return functionValue(fn);
};

@armandabric @Andarist WDYT?

[as-zlynn-philipps]

Is there a PR for this? I don't see one linked.

[as-zlynn-philipps]

OTOH - this really could be handled in the same way for any prop. People probably rarely want to see the react element's internal structure in their stringified trees

Agreed. function noRefCheck() {} isn't useful either. Nor is () => {} when it should be the element type (Comp).

Here, I'd expect onClick to be () => {} but maybe that's a Storybook actions issue.

---

EDIT: Okay, so I figured out how to configure the options for this package in Storybook via global parameters.jsx.

export const parameters = {
  jsx: {
    showFunctions: true,
    // ...
  }
}

I can set showFunctions to true there, but then I run into the problem that this issue #613 is really about. Instead of the component name, it's gobbledegook. And, since in my case, the function name is a bit different from the exported module name, that adds a layer of complexity. In my case, I might be able to leverage functionValue to achieve the desired result.

The reason onClick was showing function noRefCheck() {} is because my story uses Controls, so I'm setting the icon prop dynamically in the UI. Since sortProps is true by default, it looks like the fallback value for icon was being assigned to onClick I think? Maybe? Due to this bit in the README?

options.sortProps: boolean, default true Either to sort or not props. If you use this lib to make some isomorphic rendering you should set it to false, otherwise this would lead to react invalid checksums as the prop order is part of react isomorphic checksum algorithm.

Either way, explicitly setting sortProps to false yields a more expected result:

Just reporting my findings. ✌️

---

EDIT 2:

With the following config:

BasicUsage.parameters = {
  jsx: {
    filterProps: ['onClick'],
    functionValue: (fn: any) => fn.name.replace('Svg', ''),
    showFunctions: true,
  },
}

Voila:

Although, it's kinda jank because that functionValue would apply to any function prop. It would be neat if the prop name were passed as well, that way you could modify functionValue conditionally based on the component prop. Even better, a propValue function that takes in the prop key/value and allows you to customize the output. Related: #278, #530, #682