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

feat(Slot): mergeProps handlers should return the value of the child handler #3411

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Sheraff
Copy link

@Sheraff Sheraff commented Mar 10, 2025

Description

In some cases, we can return values from the event handlers. The best use-case I have for this is the following:

<Button
  onClick={() => mutateAsync({...})}
>

In a case like this, we can use the fact that mutateAsync returns a Promise to automatically turn on a "loading" state on our button, and turn it off when the promise resolves/rejects.

But if Slot doesn't "forward" the return value of the event child handler, this use-case becomes impossible if Button is in a Slot (that also applies an onClick handler)

<Tooltip>
  <Button onClick={() => mutateAsync({...})} />
</Tooltip>

This PR proposes that Slot should return the return value of the child handler. This makes Slot even more "transparent" to developers using it.


  • 📝 Use a meaningful title for the pull request and include the name of the package modified.
  • ✅ Add or edit tests to reflect the change (run yarn test).
  • 🔍 Add or edit Storybook examples to reflect the change (run yarn dev).
  • 🙏 Please review your own PR to check for anything you may have missed.

If you like the idea of this PR, I can try and think of tests / storybook examples

In some cases, we can return values from the event handlers. The best use-case I have for this is the following:

```tsx
<Button
  onClick={() => mutateAsync({...})}
>
```

In a case like this, we can use the fact that `mutateAsync` returns a `Promise` to automatically turn on a "loading" state on our button, and turn it off when the promise resolves/rejects.

But if `Slot` doesn't "forward" the return value of the event child handler, this use-case becomes impossible if Button is in a Slot

```tsx
<Tooltip>
  <Button onClick={() => mutateAsync({...})} />
</Tooltip>
```

This PR proposes that `Slot` should return the return value of the child handler. This makes `Slot` even more "transparent" to developers using it.
@Sheraff Sheraff changed the title feat(mergeProps): handlers should return the value of the child handler feat(Slot): mergeProps handlers should return the value of the child handler Mar 10, 2025
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