Skip to content

Add an explanation here why this.handleEvent won't cause the this t… #3500

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add an explanation here why this.handleEvent won't cause the this t… #3500

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e7f7f3
Add an explanation here why this.handleEvent won't cause the `this` t…
KennethKinLum Jan 27, 2021
713c597
Update handling-events.md
KennethKinLum Jan 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions content/docs/handling-events.md
View file
Original file line number Diff line number Diff line change
@@ -95,6 +95,8 @@ ReactDOM.render(

You have to be careful about the meaning of `this` in JSX callbacks. In JavaScript, class methods are not [bound](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_objects/Function/bind) by default. If you forget to bind `this.handleClick` and pass it to `onClick`, `this` will be `undefined` when the function is actually called.

A special note here is, `this.handleEvent` may appear in the form of `this.handleEvent()`, so it may give the impression that when `handleEvent` is invoked, the `this` is bound to the correct object. However, this is not the case, as `this.handleEvent` is first evaluated to be a reference to a function object, so it is similar to assigning `this.handleEvent` to `fn`, and then using `onClick={fn}`, and in this case, we can clearly see that the `this` will not be correctly bound.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this what the paragraphs above and below explain?

Copy link
Contributor Author

@KennethKinLum KennethKinLum Jan 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JavaScript has this property when fn is called

fn()    // first form

then the this is bound to the global object inside of fn

But when

obj.fn()    // second form

is called, then inside of fn, the this is bound to obj

In ReactJS's case, it is onClick={this.fn}

So although it looks like the second form above, it actually is the same as assigning this.fn to g, and then onClick={g}, which is actually the first form above. I am adding the note saying be careful, it is a pitfall that the form looks like obj.fn() but it actually is not the case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, isn't that what the previous and next paragraphs explain?

You have to be careful about the meaning of this in JSX callbacks. In JavaScript, class methods are not bound by default. If you forget to bind this.handleClick and pass it to onClick, this will be undefined when the function is actually called.

This is not React-specific behavior; it is a part of how functions work in JavaScript. Generally, if you refer to a method without () after it, such as onClick={this.handleClick}, you should bind that method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's because when I saw the code {this.handleClick}, since it is ReactJS specific, I didn't know how it is transpiled to. So I may think in my mind, that it is invoked as this.handleClick(), but it is not. It is actually invoked like let g = this.handleClick and then g()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's why this section explains how it is JavaScript behavior and not React specific behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that explanation is not so clear as to, if it is this.handleClick(), then it is bound correctly. If it is this.handleClick, then it is not bound correctly. There is a subtle difference there.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in your mind onClick={this.handleClick} would be transpiled to onClick: this.handleClick()? Wouldn't you also expect that it is immediately called instead of calling it only when the click happened?

I understand that this is a hard to explain topic. But I don't think the new paragraph is helping since it introduces new concepts like "the form of this.handleEvent()" and "evaluated to be a reference to a function object".

Would it maybe help to show how this somewhat looks in terms of actual code?

const componentInstance = new Toggle();
const element = { props: { onClick: componentInstance.handleClick } }

document.addEventListener('click', event => {
  const onClick = element.props.onClick;
  // Not equivalent to just `componentInstance.onClick(event)`
  onClick(event);
})

But this can be just as problematic because e.g. react event handlers aren't called with native events.

Aside: I'm sure Ricky knows how this works and how there's a difference between this.handleClick and this.handleClick(). The problem we should be focusing on is how we explain it and where we can avoid confusion.

Copy link
Contributor Author

@KennethKinLum KennethKinLum Jan 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually worked on some other projects first, and then came back to seeing onClick={this.handleClick}.
Because it was onClick={this.handleClick}, I might have thought at first it might be something that ReactJS transpiles to onclick: () => this.handleClick(). Somebody told me it is not done by addEventListener but by createElement, but either case, I might have thought of onclick: () => this.handleClick() but in fact, it is onclick: handleClick. So if it is onclick: () => this.handleClick() then it actually works, but if it is onclick: handleClick, then it doesn't work.

So I just wanted to add the pitfall and reminder that, when we see this.handleClick, don't think of it as this.handleClick(). It is just something that, if I remember this pitfall, even if I look at the code after some time, I still remember always to bind it first this.handleClick.bind(this).

So I think if I summarize it as three ways: (1) this.handleClick.bind(this) (2) () => this.handleClick() (3) handleClick = () => { ... } using arror function when it is defined in the class, then the pitfall can be avoided. I think when a potion of the reader sees onClick={this.handleClick}, whether they know if it is this.handleClick(), then it is in fact correctly bound. Or some of them may think, it in fact looks like this.handleClick(), so how come it is not bound correctly? So I just wanted to add the explanation there to say, it is in fact the same as onClick={g}.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, so it might make sense to link to the section that explains how JSX is transpiled in case people are wondering. Otherwise I don't see how we can guard against people making unfounded assumptions.

Aside: Transpiling onClick={this.handleClick} to onClick: () => this.handleClick would break memoization. So less transpilation magic is actually beneficial here.

Copy link
Contributor Author

@KennethKinLum KennethKinLum Jan 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, do you suggest adding a short explanation there or adding a link. If the readers see onclick: this.h is the same as onclick: g, and the binding is lost like that, then we have all the readers covered. Those who don't know why the lost binding can go look up other JS blog or tutorial.


This is not React-specific behavior; it is a part of [how functions work in JavaScript](https://www.smashingmagazine.com/2014/01/understanding-javascript-function-prototype-bind/). Generally, if you refer to a method without `()` after it, such as `onClick={this.handleClick}`, you should bind that method.

If calling `bind` annoys you, there are two ways you can get around this. If you are using the experimental [public class fields syntax](https://babeljs.io/docs/plugins/transform-class-properties/), you can use class fields to correctly bind callbacks: