Skip to content

Component with forwardRef passed as prop #613

Open
@tu4mo

Description

@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

Activity

armandabric

armandabric commented on Sep 28, 2021

@armandabric
Collaborator

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

Andarist commented on Sep 28, 2021

@Andarist
Contributor

It's an interesting case - this is not a valid React element so it doesn't go into this logic:

if (isValidElement(propValue)) {
return `{${formatTreeNode(
parseReactElement(propValue, options),
true,
lvl,
options
)}}`;
}

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

armandabric commented on Sep 30, 2021

@armandabric
Collaborator

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

Andarist commented on Sep 30, 2021

@Andarist
Contributor

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

armandabric commented on Sep 30, 2021

@armandabric
Collaborator

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

tu4mo commented on Sep 30, 2021

@tu4mo
Author

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

armandabric commented on Oct 6, 2021

@armandabric
Collaborator

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

Andarist

Andarist commented on Oct 7, 2021

@Andarist
Contributor

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

iMoses-Apiiro commented on Dec 24, 2021

@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

as-zlynn-philipps commented on Mar 4, 2022

@as-zlynn-philipps

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

1 remaining item

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Component with forwardRef passed as prop · Issue #613 · algolia/react-element-to-jsx-string