Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addon-docs: Fix Memo React components in ArgsTable #12686

Merged
merged 9 commits into from Oct 8, 2020

Conversation

andezzat
Copy link
Member

@andezzat andezzat commented Oct 7, 2020

Issues: #8894 #9586

Storybook seems to be missing correct exotic component type extraction.
For Forward Ref, if the wrapped component has propTypes defined, it will work, but if it doesn't while the inner render fn does, it won't work.
For Memo, it doesn't work at all.

What I did

  • Created React lib helpers for isMemo & isForwardRef (cannot use react-is counterpart as they always expect Elements not Components)
  • Refactored jsxDecorator to use new helpers & fetch .__docgen.displayName if available!
  • Fixed extractProps so it can get types from Memo render fn
  • Consolidated example exotic components
  • Updated stories to reflect new behaviour

NOTE: Unfortunately, docgen doesn't work properly if you try to attach propTypes to the wrapped Memo component, it can only work when the Memo's inner/render fn itself has propTypes attached.

How to test

  • Is this testable with Jest or Chromatic screenshots?
    No?
  • Does this need a new example in the kitchen sink apps?
    Yes, added new stories on official-storybook app.
  • Does this need an update to the documentation?
    No.

If your answer is yes to any of these, please make sure to include it in your PR.

For determining if a component isMemo or isForwardRef
jsxDecorator can now fetch component name from docgen & also using new helpers.
Consolidate forwardRef button components & fix MemoButton so its render fn has a displayName.
Now showing off that argstable works w/ Memo components & also Forward Ref w/ propTypes on render fn (inner) or on wrapped component (outer).
@andezzat andezzat requested a review from usulpro as a code owner October 7, 2020 05:55
Attaching propTypes to forwardRef component's inner render fn doesn't make sense, so we are only supporting outer propTypes. Keeping inner proptypes story to show it doesn't work & isn't recommended.
@shilman shilman added this to the 6.1 docs milestone Oct 7, 2020
@andezzat andezzat changed the title Addon-docs: Fix exotic React components in ArgsTable Addon-docs: Fix Memo React components in ArgsTable Oct 7, 2020
@andezzat andezzat self-assigned this Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants