mediamonks / frontend-coding-standards

Media.Monks - Frontend Coding Standards
60 stars 23 forks source link

handleClick or onClick? #34

Closed ThijsTyZ closed 2 years ago

ThijsTyZ commented 3 years ago

At the moment we are using both handle as on for a prefix for callbacks. We used to use handle for handlers and on for callbacks, but with the current frameworks we are using there is no longer a clear distinguish between a handler and a callback. Therefore I suggest we pick only one prefix for both callbacks and handlers.

React is using on so I would suggest we use that too. What do you think?

gianko commented 3 years ago

I like on too, is shorter and consistent onClick={onClick}

ThaNarie commented 3 years ago

Yes, with all the abstraction, the lines have become blurry, I rarely even use addEventListener anymore, so everything looks the same.

I'm in the on team now :)

psimk commented 3 years ago

IMO,

Most of the Web APIs use on as there's usually never a need to handle the event, we're just reacting to it. handle feels like it would be more useful for internal code, for example handling whether a navigation change should be prevented.

tworrisb commented 3 years ago

I think that's just over complicating things. I would just go for onClick

Miguel-Bento-Github commented 3 years ago

onClick={onClick} I find this to be detrimental for readability. We're naming an Event and a Function the exact same thing. I much rather prefer onClick={doSomething}, this way I can read code like book.

ThaNarie commented 3 years ago

I find this to be detrimental for readability. We're naming an Event and a Function the exact same thing.

onClick is not a good name, onSomethingClick would be much preferred in such cases.

I much rather prefer onClick={doSomething}

Let me try to explain why that wound't be preferred in that way exactly, but could work in a slightly modified way.

There are multiple ways to define callbacks, and multiple ways callbacks can be called.

The most used and basic form are event listeners:

function doSomething() { ... }

function onAcceptClick(event: MouseEvent) {
  // optionally do something with the event
  // do some logic
  // or call another function
  doSomething();
}

this.acceptButton.addEventListener('click', this.onAcceptClick);

Then we have your JSX example:

function doSomething() { ... }

function onAcceptClick(event: MouseEvent) {
  // optionally do something with the event
  // do some logic
  // or call another function
  doSomething();
}

return (
  <button onClick={onAcceptClick}>Accept</button>
)

And another version of your example with doSomething:

function doSomething() {
  // there is no event
  // just some logic
}

// inline function to ignore the event, since doSomething isn't an event handler/callback,
// just a normal function that can be called from other places without the event parameter
return (
  <button onClick={() => doSomething()}>Accept</button>
)

Not all callbacks are for events, some receive custom arguments, or sometimes no arguments are passed at all.

When you "bind" unrelated functions directly to callbacks, and you add parameters to the callback at a later time, your callback function might not expect this if it's also used on other places, making refactors more tricky.

When naming your functions on..., it's clear they are meant to specifically react to that event that is happening. Most of the time, the amount code in this functions is very small, and it would call other functions to delegate shared or isolated logic.

Anyhow, this is how I see it, I hope this additional context helps a bit. Perhaps some others have their on opinions based on their experiences :)

Miguel-Bento-Github commented 3 years ago

I guess this really applies to JSX and React but lets imagine Vue or Angular where the event can be passed as an argument

<element onClick=function($event) />

On top of that, I can also call other functions

eg: <element onClick="function(firstArgument, secondArgument); anotherFunction($event);" />

This way I can delegate shared and/or isolated logic.

Maybe we can align the way we write, with the previous example you gave, and in these frameworks we can also use the same mindset by taking care of all of this in the method <element onClick="onPurchaseClick($event, firstArgument, secondArgument)" /> This way all 3 different frameworks use the same logic making it easier to jump in between projects and read each others code.