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

[core] mergeReactProps cannot merge non-native event handlers #1430

Closed
mj12albert opened this issue Feb 7, 2025 · 1 comment · Fixed by #1440
Closed

[core] mergeReactProps cannot merge non-native event handlers #1430

mj12albert opened this issue Feb 7, 2025 · 1 comment · Fixed by #1440
Labels
bug 🐛 Something doesn't work core Infrastructure work going on behind the scenes

Comments

@mj12albert
Copy link
Member

Sandbox: https://codesandbox.io/p/sandbox/fervent-chaplygin-779dcw

Click the toggle and it will throw: Uncaught TypeError: Cannot create property 'preventBaseUIHandler' on boolean 'true'

<Tooltip.Trigger
  render={
    <Toggle
      pressed={isPressed}
      onPressedChange={(nextPressed) => {
        setPressed(nextPressed);
      }}
    >
      <Icon />
    </Toggle>
  }
/>

The uncontrolled counterpart of this works: https://codesandbox.io/p/sandbox/epic-hofstadter-62pnf3

The issue is that mergeReactProps expects the event as the first argument like native handlers, but our custom handlers usually pass event as the second: https://github.com/mui/base-ui/blob/master/packages/react/src/utils/mergeReactProps.ts#L44

@mj12albert mj12albert added the bug 🐛 Something doesn't work label Feb 7, 2025
@atomiks
Copy link
Contributor

atomiks commented Feb 7, 2025

This is also the case with e.g. motion events, although my solution here assumes it's an object which is also not enough:

https://github.com/mui/base-ui/pull/1236/files#diff-9c898048e86d277b9a0100de67d4d51e04b4c64bfed6c5412e6cb537ebaab3f1L52-R57

@mj12albert mj12albert changed the title [core] mergeReactProps cannot merge our custom handlers [core] mergeReactProps cannot merge non-native event handlers Feb 7, 2025
@mj12albert mj12albert added the core Infrastructure work going on behind the scenes label Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants