feat(defaultPropsHandler): Fully support forwardRef #350
Conversation
| /** | ||
| * function, lazy, memo, forwardRef etc components can resolve default props as well | ||
| */ | ||
| if (!isReactComponentClass(componentDefinition)) { | ||
| statelessProps = getStatelessPropsPath(componentDefinition); |
There was a problem hiding this comment.
Happy to change that too. This was always a bit misleading and with hooks it is definitely wrong.
There was a problem hiding this comment.
What are you referring here? Changing the name?
There was a problem hiding this comment.
Naming yes. Doesn't matter really since it's internals only. This should probably be handled in a different PR and applied consistently. But I don't know if this would change some react-docgen/utils API so I guess this is fine.
There was a problem hiding this comment.
What would you rename it to?
There was a problem hiding this comment.
I don't know. Probably just getFunctionPropsPath or just getPropsPath. Naming is hard but I don't think this discussion serves any purpose.
|
Thanks for your contribution again. Looks good to me, I just wonder can you confirm it also works when the Component is not directly nested? Like: import React from 'react';
const Component = ({ foo = 'bar' }, ref) => (<div ref={ref}>{foo}</div>);
React.forwardRef(Component);By looking at the code we might have to do another |
I'm not sure we should support this pattern since the argument for forwardRef isn't a component but a render function that takes a second argument. This may be technically only but you couldn't use const Component = (props, ref) => {}
Component.propTypes = {}; // Warning: forwardRef render functions do not support propTypes or defaultProps. Did you accidentally pass a React component?
export default React.forwardRef(Component);If you think this is too rigid I can add an additional test and see if it works. |
|
I see that makes sense. 👍 |
* test(failing): defaultPropsHandler and forwardRef * feat(defaultPropsHandler): support forwardRef # Conflicts: # src/handlers/__tests__/__snapshots__/defaultPropsHandler-test.js.snap
In
React.forwardRef(({ foo = 'bar' }, ref) => <div ref={ref}>{foo}</div>)the default props handler would not detect thatfoohad a default value.I was curious if #343 would've fixed this as well but this is not the case.