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

Support inference extra props from as props #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gtkatakura
Copy link

@gtkatakura gtkatakura commented Jul 29, 2020

What is the purpose of this pull request?

Add support to merge props from the component passed in prop as.

What problem is this solving?

Correct type detection of new props when as is passed.

How should this be manually tested?

The examples of Button.stories.tsx could have correct type.

  1. The example Checkbox has changed to compose Button with a and Link, instead compose Checkbox with Button, so the props of a and Link must be merged with the Button in this cases.
  2. The examples without defining as should merge the props of ThemeAwareButton by default.

Screenshots or example usage

Before:

Screenshot from 2020-07-29 16-01-56

After:
Screenshot from 2020-07-29 16-02-40

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Quality improvement (tests or refactors)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Trivial change (small fix or feature that doesn't impact functionalities)
  • Requires change to documentation, which has been updated accordingly

How does this PR make you feel? 🔗

Notes

I need to cast the return of forwardRef, because forwardRef don't pass the generics forward.

@@ -51,6 +51,7 @@
"react-dom": "^16.13.1",
"react-scripts": "^3.4.1",
"reakit": "^1.1.2",
"reakit-utils": "^0.13.1",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if the correct is devDependencies or peerDependencies, because I only use the types of reakit-utils here

src/Button.stories.tsx Outdated Show resolved Hide resolved
@gtkatakura gtkatakura force-pushed the master branch 2 times, most recently from 3f0e9f5 to 14f5b07 Compare July 29, 2020 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant