Only call element.on when the merchant passes in a callback

[awalker-stripe]

Summary & motivation

In PayButtonElement, merchants received an error message if they didn't handle the (optional) onClick handler about not resolving within a timeout. This was the result of our React code calling element.on to attach a event handler for every event, passing in a no-op function whenever the merchant didn't define one.

This changes our approach to only call element.on(event, callback) when the merchant actually defines a callback.

Testing & documentation

• Modified existing unit tests to make sure element.on and element.off were called at the right times.
• Manually tested to make sure PayButtonElement is able to display a payment sheet whenever onClick is left undefined
• Manually tested adding/removing onClick handlers, including switching between them being undefined to make sure element.on and element.off was working as expected.

[awalker-stripe]

r? @jackieosborn-stripe

[jackieosborn-stripe]

I'm not sure if this needs to return anything - since whatever returned in a useEffect is just the cleanup function

[faeb187]

Suggested change

	return () => {};
	return;

@jackieosborn-stripe It feels like React is obliged to remember to fire a blank onUnmount.
I don't expect React to even fire empty function bodies, but just to return quietly seems --confusion && ++readability

---

btw I dislike: element as any sorcery 🪄basically avoiding TS in a .ts file

I've started to use: SonarLint and now SonarQube

"analyse code in real time as you type in your IDE and get live feedback & guidance"

Feels promising.

[awalker-stripe]

The return () => {}; was actually to get around an eslint issue:
Arrow function expected no return value.eslint[consistent-return](https://eslint.org/docs/rules/consistent-return)

@jackieosborn-stripe: Between return; and ignoring the error, vs return () => {};, are you ok to keep as-is? (Please lmk if there's a better way around this too!)

@faeb187: Thank you for your comment! Agreed on (element as any) not being ideal (or using any almost ever). However in this case, I believe it's a reasonable tradeoff over more elaborately checking the types here, especially since calling element.on with an unsupported event isn't breaking.

[jackieosborn-stripe]

Ah... eslint. that makes sense!