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

[Button]: onClick not working in Next.js client component #10751

Open
1 task done
Mau04 opened this issue Jan 31, 2025 · 7 comments
Open
1 task done

[Button]: onClick not working in Next.js client component #10751

Mau04 opened this issue Jan 31, 2025 · 7 comments
Assignees
Labels
bug This issue is a bug in the code High Prio TOPIC Core

Comments

@Mau04
Copy link

Mau04 commented Jan 31, 2025

Describe the bug

I added a button in a client component and added an onClick handler, but the onClick handler is not called.

If I add a standard HTML button and add an onClick handler, it works. After clicking this button once, the UI5 button works, too.

Isolated Example

https://stackblitz.com/edit/github-jednsxqe?file=app%2Fpage.tsx,app%2Flayout.tsx

Reproduction steps

  1. Add a UI5 button to a client component
  2. Add an onClick handler
  3. Click the button -> nothing happens

Expected Behaviour

The handler should be called.

Screenshots or Videos

No response

UI5 Web Components for React Version

2.6.0

UI5 Web Components Version

2.6.3

Browser

Chrome

Operating System

macOS

Additional Context

No response

Relevant log output

Organization

SAP

Declaration

  • I’m not disclosing any internal or sensitive information.
@Lukas742
Copy link
Collaborator

Lukas742 commented Feb 3, 2025

Hi @Mau04

Thanks for reporting! I'll forward this issue to our UI5 Web Components Colleagues as the affected component is developed in their repository.

As a workaround, you can delay the rendering of the ui5 web component. E.g. by using requestAnimationFrame or setTimeout: https://stackblitz.com/edit/github-jednsxqe-br6n88zu?file=app%2Fpage.tsx,app%2Fbutton-test.component.tsx


Hi Colleagues,
please treat this issue with high priority as it affects all ui5 web component events in all applications using the Next.js App-router (React19).

The bug seems to be a race condition, as I wasn't able to reproduce the behavior every time, but I managed to create a setup, that should show the bug most of the time:

https://stackblitz.com/edit/github-jednsxqe-trgqrudq?file=app%2Fbutton-test.component.tsx

Reproduction steps

  1. Go to StackBlitz
  2. Press the first button. -> The button only triggers the SyntheticBaseEvent (native event wrapped by React) if onClick is passed. onclick is not registered/ignored.
  3. Click on one of the list items in the first list. -> No event is called.
  4. Press the second button which was rendered with a delay. -> see both the onClick and onclick handlers are called. The onclick event seems to be using the native event (PointerEvent) directly without being added to the React fiber tree (SyntheticBaseEvent)
  5. Press one of the list items in the second list which was rendered with a delay. -> See that both events are called (CustomEvent - selection-change and SelectionChange).
2025-02-03_12-49-09.mp4

In case you can't reproduce the issue, you can try the following:

  • Hot reload sometimes leads to rerendering, which then registers the event correctly. If you've adjusted the example, make sure to fully refresh the site.
  • Refresh the Stackblitz website, not only the example (iframe).
  • Disable cache in browser
  • Download the StackBlitz example and run it locally.

@Lukas742 Lukas742 transferred this issue from SAP/ui5-webcomponents-react Feb 3, 2025
@NHristov-sap NHristov-sap added bug This issue is a bug in the code High Prio TOPIC Core labels Feb 4, 2025
@NHristov-sap
Copy link
Contributor

Hi @ui5-webcomponents-topic-core,

Please check the reported issue, it probably affects all components.

Best Regards,
Nikolay Hristov

@pskelin pskelin self-assigned this Feb 4, 2025
@pskelin
Copy link
Contributor

pskelin commented Feb 4, 2025

Hi @Lukas742 ,
I did a lot of debugging and my conclusion is that react simply does not attach custom events when hydrating a server response.

React example
For a simple test, I rendered a tag that does not exist with an event handler:

<not-found onMyEvent={console.log}></not-found>
Image

I was expecting MyEvent above abort as it is with upper case, but it is not attached.

Preact example
I tried the same approach with preact (you can try it with npm init vite-extra and select preact-ssr from the templates.
I added the same markup to a non-existing element

<not-found onMyEvent={console.log}></not-found>
Image

Compared to react, the tag is rendered and the event handler is attached.

React debugging
Your example with rendering after an animation frame simply means that the content is rendered on the client, and by that the the normal react rendering is attaching the events. When hydrating a server response, a different logic is running that is not attaching event handlers.

I could find the starting point of this process by checking where the onClick property is handled and found it comes from hydrateRoot method, which calls

listenToAllSupportedEvents(container);

https://github.com/facebook/react/blob/0a82580bfc80538c5ce914514dc86b17c8889954/packages/react-dom/src/client/ReactDOMRoot.js#L333

This method is looping through an array called allNativeEvents and is calling listenToNativeEvent(...)
https://github.com/facebook/react/blob/0a82580bfc80538c5ce914514dc86b17c8889954/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js#L418

My conclusion is that react hydration is attaching only native event handlers by going through a hardcoded list of known browser events.

Next steps
Funny thing is the button example from the first comment starts working. I think some invalidation is running and after diffing, react gets a new function instance that is attached (but by the client rendering logic, not by the hydration logic), maybe because a state is used in the function?

I see two ways forward:

  1. open an issue in the react project
  2. try to force the client diffing logic after hydration so that the events are attached as expected, since the client diffing can attach them.

By the way, this whole investigation was for the SelectionChange event which is clearly a custom event name. For the onClick event, it doesn't work because you are technically passing onclick as a property which goes in the same "custom event not attached at hydration". If you pass from the wrappers an onClick handler, it will work via the synthetic event handling of react. Doesn't solve custom events, but will at least solve the button click.

What do you think?

@Lukas742
Copy link
Collaborator

Lukas742 commented Feb 5, 2025

Hi @pskelin

this helped a lot, thanks!

Your example with rendering after an animation frame simply means that the content is rendered on the client

You are correct, using requestAnimationFrame isn't even required there, as useEffect is only run on the client and therefore the component is only mounted on the client.

Funny thing is the button example from the first comment starts working.

Yes, I now believe this is why the issue went undetected for so long. If any React state or prop change occurs, the component updates, revalidating all props, including handler props. In @Mau04's example, the ui5-buttons start working once the native button is clicked because a state update is triggered.

Fixing the issue

We will first implement a workaround in our wrapper function by ensuring the component only renders on the client:

if(typeof window === "undefined"){
   return null;
}

Next, I'll open a GitHub issue - either with React or Next.js (as it's easier to reproduce there) - but I’m not sure yet when I’ll find the time for this.

@Lukas742
Copy link
Collaborator

Lukas742 commented Feb 5, 2025

The workaround is now live and can be consumed with v2.7.1 of @ui5/webcomponents-react.

https://stackblitz.com/edit/github-jednsxqe-xpegfhzd?file=app%2Fpage.tsx,package.json

@Mau04
Copy link
Author

Mau04 commented Feb 6, 2025

The workaround is now live and can be consumed with v2.7.1 of @ui5/webcomponents-react.

https://stackblitz.com/edit/github-jednsxqe-xpegfhzd?file=app%2Fpage.tsx,package.json

I just tried v2.7.1 in my project and now I'm running into a hydration error, because the server side rendering does not match the re-rendering on client side:

react-dom-client.development.js:4129 Uncaught Error: Hydration failed because the server rendered HTML didn't match the client. As a result this tree will be regenerated on the client. This can happen if a SSR-ed Client Component used:

- A server/client branch `if (typeof window !== 'undefined')`.
- Variable input such as `Date.now()` or `Math.random()` which changes each time it's called.
- Date formatting in a user's locale which doesn't match the server.
- External changing data without sending a snapshot of it along with the HTML.
- Invalid HTML tag nesting.

The same happens to me in your Stackblitz.

I also tried my example in Next.js 14 and it worked fine, so I guess something changed between Next.js 14 -> 15 and/or React 18 -> 19.

@Lukas742
Copy link
Collaborator

Lukas742 commented Feb 7, 2025

@Mau04 Currently, there will always be a diff between the markup on the server and the markup on the client. You can find out more about it in this issue. That's why we're using suppressHydrationWarning in our wrapper, to prevent the console from being spammed with hydration warnings.

When returning null on the server but the component on the client, a hydration error occurs, but we can't suppress it this way. Once this PR is merged and the changes published, we will no longer attach events but still add the component on the server. This has another advantage: once ui5 web components support SSR, hydration should work even with the workaround (theoretically).

I also tried my example in Next.js 14 and it worked fine, so I guess something changed between Next.js 14 -> 15 and/or React 18 -> 19.

The difference is the React version. Next.js 15 App Router is using React 19 under the hood (even before there was a stable release) and since React 19 (mostly) supports custom elements, much of our wrapper logic was removed. With React 18 this does not occur, because event listener are added by our wrapper, not by React.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug in the code High Prio TOPIC Core
Projects
Status: 🆕 New
Development

No branches or pull requests

4 participants